OFBiz › OFBiz - Dev

OFBIZ-4941

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

27 messages Options

Jacques Le Roux

OFBIZ-4941

Administrator

If nobody disagree I will commit https://issues.apache.org/jira/browse/OFBIZ-4941 soon

Jacques

Jacopo Cappellato-4

Re: OFBIZ-4941

The log history is huge and messed up and there are several files attached to the Jira task: this makes it difficult to review the work that you would like to commit.
Having a summary of the code that is going to be contributed and a final list of files/diffs would help me to take a decision... but hopefully others have been more involved in the details to express their opinion; are you also going to remove the existing help system or this new one will be added as an alternative option? In my opinion we can't have the luxury to maintain two help systems in OFBiz.

Kind regards,

Jacopo

On Nov 10, 2012, at 12:17 PM, Jacques Le Roux wrote:

> If nobody disagree I will commit https://issues.apache.org/jira/browse/OFBIZ-4941 soon
>
> Jacques

Jacques Le Roux

Re: OFBIZ-4941

Administrator

The 4/5 last comments are enough to read, before were mostly exchange between Tom and I. Adrian's concern is addressed in those last comments.
I have just attache a OFBIZ-4941.patch corresponding to what will be actually committed. This will replace the current help system and will be completed by Tom. He clearly said that he is committed to finish the work, and I trust him: most is done already.

In doubt, sorting attachments by date helps.

Thanks for your comments and opinion :)

Jacques

From: "Jacopo Cappellato" <[hidden email]>

> The log history is huge and messed up and there are several files attached to the Jira task: this makes it difficult to review the work that you would like to commit.
> Having a summary of the code that is going to be contributed and a final list of files/diffs would help me to take a decision... but hopefully others have been more involved in the details to express their opinion; are you also going to remove the existing help system or this new one will be added as an alternative option? In my opinion we can't have the luxury to maintain two help systems in OFBiz.
>
> Kind regards,
>
> Jacopo
>
>
> On Nov 10, 2012, at 12:17 PM, Jacques Le Roux wrote:
>
>> If nobody disagree I will commit https://issues.apache.org/jira/browse/OFBIZ-4941 soon
>>
>> Jacques
>
>

Jacopo Cappellato-4

Re: OFBIZ-4941

Thank you Jacques,

here is some quick feedback after a review of the patch.

A) all the code in framework/images/webapp/images/fieldlookup.js is bad because contains hardcoded application/components

B) what is this?

Index: specialpurpose/birt/ofbiz-component.xml
===================================================================
--- specialpurpose/birt/ofbiz-component.xml (revision 1407381)
+++ specialpurpose/birt/ofbiz-component.xml (working copy)
@@ -29,7 +29,6 @@
<entity-resource type="data" reader-name="seed" loader="main" location="data/OrderPortletData.xml"/>
<service-resource type="model" loader="main" location="servicedef/services.xml"/>

- 
<webapp name="birt"
title="BIRT"
server="default-server"

C) I still think it is a bad idea to add dependencies to the content component on other applications like: applications/content/webapp/ofbizhelp/catalog_en

D) did you check the compliance with licenses? See this for example:

+/*----------------------------------------------------------------------------
+ * JavaScript for webhelp search
+ *----------------------------------------------------------------------------
+ This file is part of the webhelpsearch plugin for DocBook WebHelp
+ Copyright (c) 2007-2008 NexWave Solutions All Rights Reserved.
+ www.nexwave.biz Nadege Quaine
+ http://kasunbg.blogspot.com/ Kasun Gajasinghe
+ */

E) how was generated the content of (for example):

Index: applications/content/webapp/ofbizhelp/catalog_en/content/search/htmlFileInfoList.js

? How should we maintain it?

F) why this:

applications/content/webapp/ofbizhelp/catalog_en/common/jquery/jquery-1.4.2.min.js

?

I think it is enough for now, but the changes are big and I couldn't review everything.

In general, my preference would be to see this type of contribution being implemented as a pluggable feature (with data mainatined externally in Confluence or in a specialpurpose or extra component) rather than being part of the trunk.

Kind regards,

Jacopo

On Nov 10, 2012, at 12:44 PM, Jacques Le Roux wrote:

> The 4/5 last comments are enough to read, before were mostly exchange between Tom and I. Adrian's concern is addressed in those last comments.
> I have just attache a OFBIZ-4941.patch corresponding to what will be actually committed. This will replace the current help system and will be completed by Tom. He clearly said that he is committed to finish the work, and I trust him: most is done already.
>
> In doubt, sorting attachments by date helps.
>
> Thanks for your comments and opinion :)
>
> Jacques
>
> From: "Jacopo Cappellato" <[hidden email]>
>> The log history is huge and messed up and there are several files attached to the Jira task: this makes it difficult to review the work that you would like to commit.
>> Having a summary of the code that is going to be contributed and a final list of files/diffs would help me to take a decision... but hopefully others have been more involved in the details to express their opinion; are you also going to remove the existing help system or this new one will be added as an alternative option? In my opinion we can't have the luxury to maintain two help systems in OFBiz.
>>
>> Kind regards,
>>
>> Jacopo
>>
>>
>> On Nov 10, 2012, at 12:17 PM, Jacques Le Roux wrote:
>>
>>> If nobody disagree I will commit https://issues.apache.org/jira/browse/OFBIZ-4941 soon
>>>
>>> Jacques
>>
>>

Jacques Le Roux

Re: OFBIZ-4941

Administrator

Thanks for your help Jacopo,

A) Indeed I did not review fieldlookup.js, same type of concern than in C)
B) This has been addressed by Tom and is no longer a concern
C) Yes this is the moot point, Tom gave some arguments in https://issues.apache.org/jira/browse/OFBIZ-4941?focusedCommentId=13494044&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13494044
D) Yes it's totally OK from a license POV

Jacques

From: "Jacopo Cappellato" <[hidden email]>

> Thank you Jacques,
>
> here is some quick feedback after a review of the patch.
>
> A) all the code in framework/images/webapp/images/fieldlookup.js is bad because contains hardcoded application/components
>
> B) what is this?
>
> Index: specialpurpose/birt/ofbiz-component.xml
> ===================================================================
> --- specialpurpose/birt/ofbiz-component.xml (revision 1407381)
> +++ specialpurpose/birt/ofbiz-component.xml (working copy)
> @@ -29,7 +29,6 @@
> <entity-resource type="data" reader-name="seed" loader="main" location="data/OrderPortletData.xml"/>
> <service-resource type="model" loader="main" location="servicedef/services.xml"/>
>
> - 
> <webapp name="birt"
> title="BIRT"
> server="default-server"
>
> C) I still think it is a bad idea to add dependencies to the content component on other applications like: applications/content/webapp/ofbizhelp/catalog_en
>
> D) did you check the compliance with licenses? See this for example:
>
> +/*----------------------------------------------------------------------------
> + * JavaScript for webhelp search
> + *----------------------------------------------------------------------------
> + This file is part of the webhelpsearch plugin for DocBook WebHelp
> + Copyright (c) 2007-2008 NexWave Solutions All Rights Reserved.
> + www.nexwave.biz Nadege Quaine
> + http://kasunbg.blogspot.com/ Kasun Gajasinghe
> + */
>
> E) how was generated the content of (for example):
>
> Index: applications/content/webapp/ofbizhelp/catalog_en/content/search/htmlFileInfoList.js
>
> ? How should we maintain it?
>
> F) why this:
>
> applications/content/webapp/ofbizhelp/catalog_en/common/jquery/jquery-1.4.2.min.js
>
> ?
>
> I think it is enough for now, but the changes are big and I couldn't review everything.
>
> In general, my preference would be to see this type of contribution being implemented as a pluggable feature (with data mainatined externally in Confluence or in a specialpurpose or extra component) rather than being part of the trunk.
>
> Kind regards,
>
> Jacopo
>
>
> On Nov 10, 2012, at 12:44 PM, Jacques Le Roux wrote:
>
>> The 4/5 last comments are enough to read, before were mostly exchange between Tom and I. Adrian's concern is addressed in those last comments.
>> I have just attache a OFBIZ-4941.patch corresponding to what will be actually committed. This will replace the current help system and will be completed by Tom. He clearly said that he is committed to finish the work, and I trust him: most is done already.
>>
>> In doubt, sorting attachments by date helps.
>>
>> Thanks for your comments and opinion :)
>>
>> Jacques
>>
>> From: "Jacopo Cappellato" <[hidden email]>
>>> The log history is huge and messed up and there are several files attached to the Jira task: this makes it difficult to review the work that you would like to commit.
>>> Having a summary of the code that is going to be contributed and a final list of files/diffs would help me to take a decision... but hopefully others have been more involved in the details to express their opinion; are you also going to remove the existing help system or this new one will be added as an alternative option? In my opinion we can't have the luxury to maintain two help systems in OFBiz.
>>>
>>> Kind regards,
>>>
>>> Jacopo
>>>
>>>
>>> On Nov 10, 2012, at 12:17 PM, Jacques Le Roux wrote:
>>>
>>>> If nobody disagree I will commit https://issues.apache.org/jira/browse/OFBIZ-4941 soon
>>>>
>>>> Jacques
>>>
>>>
>
>

Jacopo Cappellato-4

Re: OFBIZ-4941

On Nov 10, 2012, at 3:11 PM, Jacques Le Roux wrote:

> Thanks for your help Jacopo,
>
> A) Indeed I did not review fieldlookup.js, same type of concern than in C)
> B) This has been addressed by Tom and is no longer a concern

Do you mean that the code will not be committed?

> C) Yes this is the moot point, Tom gave some arguments in https://issues.apache.org/jira/browse/OFBIZ-4941?focusedCommentId=13494044&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13494044

Tom's answer doesn't really address Adrian's remark; instead of adding them to "content" I would rather prefer to have a new (specialpurpose/extra) component containing all the help files.

> D) Yes it's totally OK from a license POV

I don't see the updates to the LICENSE file in your patch... these changes will help me to better understand how the files are treated from a licensing point of view

Kind regards,

Jacopo

>
> Jacques
>
> From: "Jacopo Cappellato" <[hidden email]>
>> Thank you Jacques,
>>
>> here is some quick feedback after a review of the patch.
>>
>> A) all the code in framework/images/webapp/images/fieldlookup.js is bad because contains hardcoded application/components
>>
>> B) what is this?
>>
>> Index: specialpurpose/birt/ofbiz-component.xml
>> ===================================================================
>> --- specialpurpose/birt/ofbiz-component.xml (revision 1407381)
>> +++ specialpurpose/birt/ofbiz-component.xml (working copy)
>> @@ -29,7 +29,6 @@
>> <entity-resource type="data" reader-name="seed" loader="main" location="data/OrderPortletData.xml"/>
>> <service-resource type="model" loader="main" location="servicedef/services.xml"/>
>>
>> - 
>> <webapp name="birt"
>> title="BIRT"
>> server="default-server"
>>
>> C) I still think it is a bad idea to add dependencies to the content component on other applications like: applications/content/webapp/ofbizhelp/catalog_en
>>
>> D) did you check the compliance with licenses? See this for example:
>>
>> +/*----------------------------------------------------------------------------
>> + * JavaScript for webhelp search
>> + *----------------------------------------------------------------------------
>> + This file is part of the webhelpsearch plugin for DocBook WebHelp
>> + Copyright (c) 2007-2008 NexWave Solutions All Rights Reserved.
>> + www.nexwave.biz Nadege Quaine
>> + http://kasunbg.blogspot.com/ Kasun Gajasinghe
>> + */
>>
>> E) how was generated the content of (for example):
>>
>> Index: applications/content/webapp/ofbizhelp/catalog_en/content/search/htmlFileInfoList.js
>>
>> ? How should we maintain it?
>>
>> F) why this:
>>
>> applications/content/webapp/ofbizhelp/catalog_en/common/jquery/jquery-1.4.2.min.js
>>
>> ?
>>
>> I think it is enough for now, but the changes are big and I couldn't review everything.
>>
>> In general, my preference would be to see this type of contribution being implemented as a pluggable feature (with data mainatined externally in Confluence or in a specialpurpose or extra component) rather than being part of the trunk.
>>
>> Kind regards,
>>
>> Jacopo
>>
>>
>> On Nov 10, 2012, at 12:44 PM, Jacques Le Roux wrote:
>>
>>> The 4/5 last comments are enough to read, before were mostly exchange between Tom and I. Adrian's concern is addressed in those last comments.
>>> I have just attache a OFBIZ-4941.patch corresponding to what will be actually committed. This will replace the current help system and will be completed by Tom. He clearly said that he is committed to finish the work, and I trust him: most is done already.
>>>
>>> In doubt, sorting attachments by date helps.
>>>
>>> Thanks for your comments and opinion :)
>>>
>>> Jacques
>>>
>>> From: "Jacopo Cappellato" <[hidden email]>
>>>> The log history is huge and messed up and there are several files attached to the Jira task: this makes it difficult to review the work that you would like to commit.
>>>> Having a summary of the code that is going to be contributed and a final list of files/diffs would help me to take a decision... but hopefully others have been more involved in the details to express their opinion; are you also going to remove the existing help system or this new one will be added as an alternative option? In my opinion we can't have the luxury to maintain two help systems in OFBiz.
>>>>
>>>> Kind regards,
>>>>
>>>> Jacopo
>>>>
>>>>
>>>> On Nov 10, 2012, at 12:17 PM, Jacques Le Roux wrote:
>>>>
>>>>> If nobody disagree I will commit https://issues.apache.org/jira/browse/OFBIZ-4941 soon
>>>>>
>>>>> Jacques
>>>>
>>>>
>>
>>

Adrian Crum-3

Re: OFBIZ-4941

In reply to this post by Jacopo Cappellato-4

On 11/10/2012 1:48 PM, Jacopo Cappellato wrote:

Thanks Jacopo.

This has been my position all along. Help links should point to a URL
that is retrieved from the UI labels file (to support i18n). That way
Help content can be located anywhere - inside or outside OFBiz. If an
application wants to use the OFBiz Content application to implement
Help, then it is free to do so.

-Adrian

hans_bakker

Re: OFBIZ-4941

In reply to this post by Jacopo Cappellato-4

-1

On 11/10/2012 09:27 PM, Jacopo Cappellato wrote:
> Tom's answer doesn't really address Adrian's remark; instead of adding them to "content" I would rather prefer to have a new (specialpurpose/extra) component containing all the help files.
>

hans_bakker

Re: OFBIZ-4941

reason: moving anything to special purpose will finally move to extra's
to which i completely oppose to.

Regards,
Hans

On 11/10/2012 10:22 PM, Hans Bakker wrote:
> -1
>
> On 11/10/2012 09:27 PM, Jacopo Cappellato wrote:
>> Tom's answer doesn't really address Adrian's remark; instead of
>> adding them to "content" I would rather prefer to have a new
>> (specialpurpose/extra) component containing all the help files.
>>
>

Jacques Le Roux

Re: OFBIZ-4941

Administrator

In reply to this post by Jacques Le Roux

Oops I missed points after D), I will answer to them now

E) It's copied by the xsl transformation from applications/content/template/docbook/webhelp/docs/content/search/htmlFileInfoList.js wich is. actually part of Docbook version 1.77.1 (committed at r1395307)
So maintenance worries

F) it's also part of the Docbook version 1.77.1

From: "Jacques Le Roux" <[hidden email]>

> Thanks for your help Jacopo,
>
> A) Indeed I did not review fieldlookup.js, same type of concern than in C)
> B) This has been addressed by Tom and is no longer a concern
> C) Yes this is the moot point, Tom gave some arguments in https://issues.apache.org/jira/browse/OFBIZ-4941?focusedCommentId=13494044&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13494044
> D) Yes it's totally OK from a license POV
>
> Jacques
>
> From: "Jacopo Cappellato" <[hidden email]>
>> Thank you Jacques,
>>
>> here is some quick feedback after a review of the patch.
>>
>> A) all the code in framework/images/webapp/images/fieldlookup.js is bad because contains hardcoded application/components
>>
>> B) what is this?
>>
>> Index: specialpurpose/birt/ofbiz-component.xml
>> ===================================================================
>> --- specialpurpose/birt/ofbiz-component.xml (revision 1407381)
>> +++ specialpurpose/birt/ofbiz-component.xml (working copy)
>> @@ -29,7 +29,6 @@
>> <entity-resource type="data" reader-name="seed" loader="main" location="data/OrderPortletData.xml"/>
>> <service-resource type="model" loader="main" location="servicedef/services.xml"/>
>>
>> - 
>> <webapp name="birt"
>> title="BIRT"
>> server="default-server"
>>
>> C) I still think it is a bad idea to add dependencies to the content component on other applications like: applications/content/webapp/ofbizhelp/catalog_en
>>
>> D) did you check the compliance with licenses? See this for example:
>>
>> +/*----------------------------------------------------------------------------
>> + * JavaScript for webhelp search
>> + *----------------------------------------------------------------------------
>> + This file is part of the webhelpsearch plugin for DocBook WebHelp
>> + Copyright (c) 2007-2008 NexWave Solutions All Rights Reserved.
>> + www.nexwave.biz Nadege Quaine
>> + http://kasunbg.blogspot.com/ Kasun Gajasinghe
>> + */
>>
>> E) how was generated the content of (for example):
>>
>> Index: applications/content/webapp/ofbizhelp/catalog_en/content/search/htmlFileInfoList.js
>>
>> ? How should we maintain it?
>>
>> F) why this:
>>
>> applications/content/webapp/ofbizhelp/catalog_en/common/jquery/jquery-1.4.2.min.js
>>
>> ?
>>
>> I think it is enough for now, but the changes are big and I couldn't review everything.
>>
>> In general, my preference would be to see this type of contribution being implemented as a pluggable feature (with data mainatined externally in Confluence or in a specialpurpose or extra component) rather than being part of the trunk.
>>
>> Kind regards,
>>
>> Jacopo
>>
>>
>> On Nov 10, 2012, at 12:44 PM, Jacques Le Roux wrote:
>>
>>> The 4/5 last comments are enough to read, before were mostly exchange between Tom and I. Adrian's concern is addressed in those last comments.
>>> I have just attache a OFBIZ-4941.patch corresponding to what will be actually committed. This will replace the current help system and will be completed by Tom. He clearly said that he is committed to finish the work, and I trust him: most is done already.
>>>
>>> In doubt, sorting attachments by date helps.
>>>
>>> Thanks for your comments and opinion :)
>>>
>>> Jacques
>>>
>>> From: "Jacopo Cappellato" <[hidden email]>
>>>> The log history is huge and messed up and there are several files attached to the Jira task: this makes it difficult to review the work that you would like to commit.
>>>> Having a summary of the code that is going to be contributed and a final list of files/diffs would help me to take a decision... but hopefully others have been more involved in the details to express their opinion; are you also going to remove the existing help system or this new one will be added as an alternative option? In my opinion we can't have the luxury to maintain two help systems in OFBiz.
>>>>
>>>> Kind regards,
>>>>
>>>> Jacopo
>>>>
>>>>
>>>> On Nov 10, 2012, at 12:17 PM, Jacques Le Roux wrote:
>>>>
>>>>> If nobody disagree I will commit https://issues.apache.org/jira/browse/OFBIZ-4941 soon
>>>>>
>>>>> Jacques
>>>>
>>>>
>>
>>
>

Jacques Le Roux

Re: OFBIZ-4941

Administrator

In reply to this post by Jacopo Cappellato-4

From: "Jacopo Cappellato" <[hidden email]>
> On Nov 10, 2012, at 3:11 PM, Jacques Le Roux wrote:
>
>> Thanks for your help Jacopo,
>>
>> A) Indeed I did not review fieldlookup.js, same type of concern than in C)
>> B) This has been addressed by Tom and is no longer a concern
>
> Do you mean that the code will not be committed?

I commited a change with http://svn.apache.org/viewvc?view=revision&revision=1400463
<<jleroux: while checking I found also an error in birt/documents/Birt.xml and while at it I also commit a tiny part of OFBIZ-4941 in birt/ofbiz-component.xml because it breaks the help.>>
Tom's new help address this concern and we can remove the comments in this part, yes this part of the code will be committed if we finally agree about it. It already used some part of my time, but I think it's worth it on the long term...

>> C) Yes this is the moot point, Tom gave some arguments in https://issues.apache.org/jira/browse/OFBIZ-4941?focusedCommentId=13494044&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13494044
>
> Tom's answer doesn't really address Adrian's remark; instead of adding them to "content" I would rather prefer to have a new (specialpurpose/extra) component containing all the help files.

Extra does not fit: this is supposed to replace the old help and should stay as part of OFBiz OOTB. I'm not against a specialpurpose help component, if content component is not mandatory for building the help...

>> D) Yes it's totally OK from a license POV
>
> I don't see the updates to the LICENSE file in your patch... these changes will help me to better understand how the files are treated from a licensing point of view

Right, I did not address that yet, should be straightforward

Jacques

> Kind regards,
>
> Jacopo
>
>>
>> Jacques
>>
>> From: "Jacopo Cappellato" <[hidden email]>
>>> Thank you Jacques,
>>>
>>> here is some quick feedback after a review of the patch.
>>>
>>> A) all the code in framework/images/webapp/images/fieldlookup.js is bad because contains hardcoded application/components
>>>
>>> B) what is this?
>>>
>>> Index: specialpurpose/birt/ofbiz-component.xml
>>> ===================================================================
>>> --- specialpurpose/birt/ofbiz-component.xml (revision 1407381)
>>> +++ specialpurpose/birt/ofbiz-component.xml (working copy)
>>> @@ -29,7 +29,6 @@
>>> <entity-resource type="data" reader-name="seed" loader="main" location="data/OrderPortletData.xml"/>
>>> <service-resource type="model" loader="main" location="servicedef/services.xml"/>
>>>
>>> - 
>>> <webapp name="birt"
>>> title="BIRT"
>>> server="default-server"
>>>
>>> C) I still think it is a bad idea to add dependencies to the content component on other applications like: applications/content/webapp/ofbizhelp/catalog_en
>>>
>>> D) did you check the compliance with licenses? See this for example:
>>>
>>> +/*----------------------------------------------------------------------------
>>> + * JavaScript for webhelp search
>>> + *----------------------------------------------------------------------------
>>> + This file is part of the webhelpsearch plugin for DocBook WebHelp
>>> + Copyright (c) 2007-2008 NexWave Solutions All Rights Reserved.
>>> + www.nexwave.biz Nadege Quaine
>>> + http://kasunbg.blogspot.com/ Kasun Gajasinghe
>>> + */
>>>
>>> E) how was generated the content of (for example):
>>>
>>> Index: applications/content/webapp/ofbizhelp/catalog_en/content/search/htmlFileInfoList.js
>>>
>>> ? How should we maintain it?
>>>
>>> F) why this:
>>>
>>> applications/content/webapp/ofbizhelp/catalog_en/common/jquery/jquery-1.4.2.min.js
>>>
>>> ?
>>>
>>> I think it is enough for now, but the changes are big and I couldn't review everything.
>>>
>>> In general, my preference would be to see this type of contribution being implemented as a pluggable feature (with data mainatined externally in Confluence or in a specialpurpose or extra component) rather than being part of the trunk.
>>>
>>> Kind regards,
>>>
>>> Jacopo
>>>
>>>
>>> On Nov 10, 2012, at 12:44 PM, Jacques Le Roux wrote:
>>>
>>>> The 4/5 last comments are enough to read, before were mostly exchange between Tom and I. Adrian's concern is addressed in those last comments.
>>>> I have just attache a OFBIZ-4941.patch corresponding to what will be actually committed. This will replace the current help system and will be completed by Tom. He clearly said that he is committed to finish the work, and I trust him: most is done already.
>>>>
>>>> In doubt, sorting attachments by date helps.
>>>>
>>>> Thanks for your comments and opinion :)
>>>>
>>>> Jacques
>>>>
>>>> From: "Jacopo Cappellato" <[hidden email]>
>>>>> The log history is huge and messed up and there are several files attached to the Jira task: this makes it difficult to review the work that you would like to commit.
>>>>> Having a summary of the code that is going to be contributed and a final list of files/diffs would help me to take a decision... but hopefully others have been more involved in the details to express their opinion; are you also going to remove the existing help system or this new one will be added as an alternative option? In my opinion we can't have the luxury to maintain two help systems in OFBiz.
>>>>>
>>>>> Kind regards,
>>>>>
>>>>> Jacopo
>>>>>
>>>>>
>>>>> On Nov 10, 2012, at 12:17 PM, Jacques Le Roux wrote:
>>>>>
>>>>>> If nobody disagree I will commit https://issues.apache.org/jira/browse/OFBIZ-4941 soon
>>>>>>
>>>>>> Jacques
>>>>>
>>>>>
>>>
>>>
>
>

Jacques Le Roux

Re: OFBIZ-4941

Administrator

In reply to this post by Adrian Crum-3

From: "Adrian Crum" <[hidden email]>

> On 11/10/2012 1:48 PM, Jacopo Cappellato wrote:
>> Thank you Jacques,
>>
>> here is some quick feedback after a review of the patch.
>>
>> A) all the code in framework/images/webapp/images/fieldlookup.js is bad because contains hardcoded application/components
>>
>> B) what is this?
>>
>> Index: specialpurpose/birt/ofbiz-component.xml
>> ===================================================================
>> --- specialpurpose/birt/ofbiz-component.xml (revision 1407381)
>> +++ specialpurpose/birt/ofbiz-component.xml (working copy)
>> @@ -29,7 +29,6 @@
>> <entity-resource type="data" reader-name="seed" loader="main" location="data/OrderPortletData.xml"/>
>> <service-resource type="model" loader="main" location="servicedef/services.xml"/>
>>
>> - 
>> <webapp name="birt"
>> title="BIRT"
>> server="default-server"
>>
>> C) I still think it is a bad idea to add dependencies to the content component on other applications like: applications/content/webapp/ofbizhelp/catalog_en
>>
>> D) did you check the compliance with licenses? See this for example:
>>
>> +/*----------------------------------------------------------------------------
>> + * JavaScript for webhelp search
>> + *----------------------------------------------------------------------------
>> + This file is part of the webhelpsearch plugin for DocBook WebHelp
>> + Copyright (c) 2007-2008 NexWave Solutions All Rights Reserved.
>> + www.nexwave.biz Nadege Quaine
>> + http://kasunbg.blogspot.com/ Kasun Gajasinghe
>> + */
>>
>> E) how was generated the content of (for example):
>>
>> Index: applications/content/webapp/ofbizhelp/catalog_en/content/search/htmlFileInfoList.js
>>
>> ? How should we maintain it?
>>
>> F) why this:
>>
>> applications/content/webapp/ofbizhelp/catalog_en/common/jquery/jquery-1.4.2.min.js
>>
>> ?
>>
>> I think it is enough for now, but the changes are big and I couldn't review everything.
>>
>> In general, my preference would be to see this type of contribution being implemented as a pluggable feature (with data mainatined externally in Confluence or in a specialpurpose or extra component) rather than being part of the trunk.
>
> Thanks Jacopo.
>
> This has been my position all along. Help links should point to a URL
> that is retrieved from the UI labels file (to support i18n). That way
> Help content can be located anywhere - inside or outside OFBiz. If an
> application wants to use the OFBiz Content application to implement
> Help, then it is free to do so.
>
> -Adrian

Could you elaborate a bit more Adrian? You mean to put links/URLs in UI label files?
How this would relate to the current WIP for the new help?

Jacques

Adrian Crum-3

Re: OFBIZ-4941

On 11/10/2012 6:07 PM, Jacques Le Roux wrote:

> From: "Adrian Crum" <[hidden email]>
>> On 11/10/2012 1:48 PM, Jacopo Cappellato wrote:
>>> Thank you Jacques,
>>>
>>> here is some quick feedback after a review of the patch.
>>>
>>> A) all the code in framework/images/webapp/images/fieldlookup.js is bad because contains hardcoded application/components
>>>
>>> B) what is this?
>>>
>>> Index: specialpurpose/birt/ofbiz-component.xml
>>> ===================================================================
>>> --- specialpurpose/birt/ofbiz-component.xml (revision 1407381)
>>> +++ specialpurpose/birt/ofbiz-component.xml (working copy)
>>> @@ -29,7 +29,6 @@
>>> <entity-resource type="data" reader-name="seed" loader="main" location="data/OrderPortletData.xml"/>
>>> <service-resource type="model" loader="main" location="servicedef/services.xml"/>
>>>
>>> - 
>>> <webapp name="birt"
>>> title="BIRT"
>>> server="default-server"
>>>
>>> C) I still think it is a bad idea to add dependencies to the content component on other applications like: applications/content/webapp/ofbizhelp/catalog_en
>>>
>>> D) did you check the compliance with licenses? See this for example:
>>>
>>> +/*----------------------------------------------------------------------------
>>> + * JavaScript for webhelp search
>>> + *----------------------------------------------------------------------------
>>> + This file is part of the webhelpsearch plugin for DocBook WebHelp
>>> + Copyright (c) 2007-2008 NexWave Solutions All Rights Reserved.
>>> + www.nexwave.biz Nadege Quaine
>>> + http://kasunbg.blogspot.com/ Kasun Gajasinghe
>>> + */
>>>
>>> E) how was generated the content of (for example):
>>>
>>> Index: applications/content/webapp/ofbizhelp/catalog_en/content/search/htmlFileInfoList.js
>>>
>>> ? How should we maintain it?
>>>
>>> F) why this:
>>>
>>> applications/content/webapp/ofbizhelp/catalog_en/common/jquery/jquery-1.4.2.min.js
>>>
>>> ?
>>>
>>> I think it is enough for now, but the changes are big and I couldn't review everything.
>>>
>>> In general, my preference would be to see this type of contribution being implemented as a pluggable feature (with data mainatined externally in Confluence or in a specialpurpose or extra component) rather than being part of the trunk.
>> Thanks Jacopo.
>>
>> This has been my position all along. Help links should point to a URL
>> that is retrieved from the UI labels file (to support i18n). That way
>> Help content can be located anywhere - inside or outside OFBiz. If an
>> application wants to use the OFBiz Content application to implement
>> Help, then it is free to do so.
>>
>> -Adrian
> Could you elaborate a bit more Adrian? You mean to put links/URLs in UI label files?
> How this would relate to the current WIP for the new help?

See WorkEffortUiLabels.xml, key="WorkEffortICalendarHelpUrl". The link
doesn't work any more because the Wiki site changed, and I can't find
the new page. But when it was working, iCalendar help was retrieved from
the Wiki site. This arrangement did not create any dependencies on any
other components.

-Adrian

Tom

Re: OFBIZ-4941

In reply to this post by Jacopo Cappellato-4

Jacopo,

A) The hard code in fieldlookup.js will go away when all forms are documented. That will leave the code that test for locale. I'm open for suggestions on how that could be done in some other way but given what I see as the superiority of the proposed solution I do not think this should be a show stopper.

B) Code was added in ofbiz-component.xml when BIRT was moved to specialpurpose. That code caused the current help to break. The latest code for the proposed system is not effected by ofbiz-component.xml. This is because it is at the webapp not component level. ofbiz-component.xml can revert back to what it was.

Note: If you do not use the proposed system you will need to go back and do something with the BIRT ofbiz-component.xml (which needs work in any case). It causes birt help to be invoked when you try to open accounting help (or any of the other components mounted in BIRT).

C) All the help was placed in the content component because it was the home for a subset of the docbook xls distribution. It made sense to replace that code with the latest implementation and keep everything in one place rather then do something in special purpose or hot deploy with a duplicate xls code. It also makes sense since help is content and not an application. I do not see how moving the content to the application will make it independent. Are you going to duplicate the docbook distribution in each application?

D) Covered by Jacques

E) As Jacques noted it is transformed by ant using docbook xls. I do not understand his comment "So maintenance worries"

F) It is loaded by the docbook xsl webhelp web application which is the heart of the system. webhelp makes extensive use of jquery.

Also
Re: "In my opinion we can't have the luxury to maintain two help systems in
OFBiz."
True - when I have updated as promised the current system can and should be removed. That said the code will support both systems during transition.

Re: "Help links should point to a URL that is retrieved from the UI labels file (to support i18n). That way Help content can be located anywhere - inside or outside OFBiz."

Help is invoked using a mechanism similar to the current system (themes). It supports i18n in some logic in fieldlookup.js as mentioned in (A). All of the help files are compiled HTML (unlike the current system which is transformed at runtime). They could just as easily be deployed to the OFBiz site or any other web site. They are also easily transformed into pdf and can be posted anywhere.

I look forward to any other questions or comments you may have.

Tom

Tom

Re: OFBIZ-4941

In reply to this post by Adrian Crum-3

Adrian,

Your example may not depend on another component but is did depend on a file that was outside of the OFBiz app control and broke when that file changed. This will never happen in the proposed solution. All files for a webapp are generated from a single docbook file so there are no external dependencies.

You can have external links in the webhelp files and there are examples in the current upload. In the solution each locale has it's own file and would point to an appropriate external reference for that locale. Naturally if that external reference changes the link will break.

I searched the trunk for http in *UiLabels.xml and there are 120 matches. I found only one of them that references more then one locale (CommonUiLabels.xml CommonLookupAnywhoLink), so as a practical matter this is a solution that has not been used very much.

Help in the proposed system is at the webapp level. UiLabels are more (and I would argue too) fine grained for screen level context help. UI labels can be used any where pointing to any thing. It would be very hard to maintain a coherent help system using all the fragmented files that could spring from labels.

This is not to say that there is not a place for labels in cases like CommonLookupAnywhoLink. The two mechanisms can co-exist but I think they serve different purposes. A system like webhelp is better suited for creating help documents for which it was designed while UiLabels were designed for the purpose of providing i18n support for lables.

Tom

Jacques Le Roux

Re: OFBIZ-4941

Administrator

In reply to this post by Tom

From: "Tom" <[hidden email]>
> E) As Jacques noted it is transformed by ant using docbook xls. I do not
> understand his comment "So maintenance worries"

Typo, meant "So no maintenance worries"

Jacques

Jacques Le Roux

Re: OFBIZ-4941

Administrator

In reply to this post by Tom

When I look at the current state of the wiki I tend to agree with Tom...

Jacques

From: "Tom" <[hidden email]>

> Adrian,
>
> Your example may not depend on another component but is did depend on a file
> that was outside of the OFBiz app control and broke when that file changed.
> This will never happen in the proposed solution. All files for a webapp are
> generated from a single docbook file so there are no external dependencies.
>
> You can have external links in the webhelp files and there are examples in
> the current upload. In the solution each locale has it's own file and would
> point to an appropriate external reference for that locale. Naturally if
> that external reference changes the link will break.
>
> I searched the trunk for http in *UiLabels.xml and there are 120 matches. I
> found only one of them that references more then one locale
> (CommonUiLabels.xml CommonLookupAnywhoLink), so as a practical matter this
> is a solution that has not been used very much.
>
> Help in the proposed system is at the webapp level. UiLabels are more (and I
> would argue too) fine grained for screen level context help. UI labels can
> be used any where pointing to any thing. It would be very hard to maintain a
> coherent help system using all the fragmented files that could spring from
> labels.
>
> This is not to say that there is not a place for labels in cases like
> CommonLookupAnywhoLink. The two mechanisms can co-exist but I think they
> serve different purposes. A system like webhelp is better suited for
> creating help documents for which it was designed while UiLabels were
> designed for the purpose of providing i18n support for lables.
>
> Tom
>
>
>
> --
> View this message in context: http://ofbiz.135035.n4.nabble.com/OFBIZ-4941-tp4637418p4637442.html
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.

Jacques Le Roux

Re: OFBIZ-4941

Administrator

In reply to this post by Tom

From: "Tom" <[hidden email]>
> Jacopo,
>
> A) The hard code in fieldlookup.js will go away when all forms are
> documented. That will leave the code that test for locale. I'm open for
> suggestions on how that could be done in some other way but given what I see
> as the superiority of the proposed solution I do not think this should be a
> show stopper.

I agree, we already discussed this with Tom but I forgot.

Yes, thanks for https://issues.apache.org/jira/browse/OFBIZ-5070 Tom

Jacques

Anil Patel-3

Re: OFBIZ-4941

In reply to this post by Tom

Tom,

On Nov 10, 2012, at 2:51 PM, Tom <[hidden email]> wrote:

> Jacopo,
>
> A) The hard code in fieldlookup.js will go away when all forms are
> documented. That will leave the code that test for locale. I'm open for
> suggestions on how that could be done in some other way but given what I see
> as the superiority of the proposed solution I do not think this should be a
> show stopper.
>

I hate to use Javascript more then needed. In this case, We can build link to help document during the screen rendering process. We need something similar to OfbizUrl transform.

> B) Code was added in ofbiz-component.xml when BIRT was moved to
> specialpurpose. That code caused the current help to break. The latest code
> for the proposed system is not effected by ofbiz-component.xml. This is
> because it is at the webapp not component level. ofbiz-component.xml can
> revert back to what it was.
>
> Note: If you do not use the proposed system you will need to go back and do
> something with the BIRT ofbiz-component.xml (which needs work in any case).
> It causes birt help to be invoked when you try to open accounting help (or
> any of the other components mounted in BIRT).
>
> C) All the help was placed in the content component because it was the home
> for a subset of the docbook xls distribution. It made sense to replace that
> code with the latest implementation and keep everything in one place rather
> then do something in special purpose or hot deploy with a duplicate xls
> code. It also makes sense since help is content and not an application. I do
> not see how moving the content to the application will make it independent.
> Are you going to duplicate the docbook distribution in each application?

I am little confused here.

How is proposed help system using Ofbiz CMS?

>
> D) Covered by Jacques
>
> E) As Jacques noted it is transformed by ant using docbook xls. I do not
> understand his comment "So maintenance worries"
>
> F) It is loaded by the docbook xsl webhelp web application which is the
> heart of the system. webhelp makes extensive use of jquery.
>
> Also
> Re: "In my opinion we can't have the luxury to maintain two help systems in
> OFBiz."
> True - when I have updated as promised the current system can and should be
> removed. That said the code will support both systems during transition.
>
> Re: "Help links should point to a URL that is retrieved from the UI labels
> file (to support i18n). That way Help content can be located anywhere -
> inside or outside OFBiz."
>
> Help is invoked using a mechanism similar to the current system (themes). It
> supports i18n in some logic in fieldlookup.js as mentioned in (A). All of
> the help files are compiled HTML (unlike the current system which is
> transformed at runtime). They could just as easily be deployed to the OFBiz
> site or any other web site. They are also easily transformed into pdf and
> can be posted anywhere.
>
> I look forward to any other questions or comments you may have.
>
> Tom
>
>
>
>
> --
> View this message in context: http://ofbiz.135035.n4.nabble.com/OFBIZ-4941-tp4637418p4637441.html
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.

Anil Patel-3

Re: OFBIZ-4941

In reply to this post by Adrian Crum-3

On Nov 10, 2012, at 9:29 AM, Adrian Crum <[hidden email]> wrote:

> On 11/10/2012 1:48 PM, Jacopo Cappellato wrote:
>> Thank you Jacques,
>>
>> here is some quick feedback after a review of the patch.
>>
>> A) all the code in framework/images/webapp/images/fieldlookup.js is bad because contains hardcoded application/components
>>
>> B) what is this?
>>
>> Index: specialpurpose/birt/ofbiz-component.xml
>> ===================================================================
>> --- specialpurpose/birt/ofbiz-component.xml (revision 1407381)
>> +++ specialpurpose/birt/ofbiz-component.xml (working copy)
>> @@ -29,7 +29,6 @@
>> <entity-resource type="data" reader-name="seed" loader="main" location="data/OrderPortletData.xml"/>
>> <service-resource type="model" loader="main" location="servicedef/services.xml"/>
>> - 
>> <webapp name="birt"
>> title="BIRT"
>> server="default-server"
>>
>> C) I still think it is a bad idea to add dependencies to the content component on other applications like: applications/content/webapp/ofbizhelp/catalog_en
>>
>> D) did you check the compliance with licenses? See this for example:
>>
>> +/*----------------------------------------------------------------------------
>> + * JavaScript for webhelp search
>> + *----------------------------------------------------------------------------
>> + This file is part of the webhelpsearch plugin for DocBook WebHelp
>> + Copyright (c) 2007-2008 NexWave Solutions All Rights Reserved.
>> + www.nexwave.biz Nadege Quaine
>> + http://kasunbg.blogspot.com/ Kasun Gajasinghe
>> + */
>>
>> E) how was generated the content of (for example):
>>
>> Index: applications/content/webapp/ofbizhelp/catalog_en/content/search/htmlFileInfoList.js
>>
>> ? How should we maintain it?
>>
>> F) why this:
>>
>> applications/content/webapp/ofbizhelp/catalog_en/common/jquery/jquery-1.4.2.min.js
>>
>> ?
>>
>> I think it is enough for now, but the changes are big and I couldn't review everything.
>>
>> In general, my preference would be to see this type of contribution being implemented as a pluggable feature (with data mainatined externally in Confluence or in a specialpurpose or extra component) rather than being part of the trunk.
>
> Thanks Jacopo.
>
> This has been my position all along. Help links should point to a URL that is retrieved from the UI labels file (to support i18n). That way Help content can be located anywhere - inside or outside OFBiz. If an application wants to use the OFBiz Content application to implement Help, then it is free to do so.
>

+1

We should be able to very easily override help content.

> -Adrian
>