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" <jacopo.cappell...@hotwaxmedia.com> >> 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"/> >> >> - <!-- use when reports need to be injected into applications Note: this >> will break context help for those applications. >> <webapp name="accounting" >> title="Accounting" >> server="default-server" >> @@ -50,7 +49,6 @@ >> location="webapp/ordermgr" >> base-permission="OFBTOOLS,ORDERMGR" >> mount-point="/ordermgr"/> >> - --> >> <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" <jacopo.cappell...@hotwaxmedia.com> >>>> 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 >>>> >>>> >> >>
