From: "Jacopo Cappellato" <[email protected]> > 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" <[email protected]> >>> 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" <[email protected]> >>>>> 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 >>>>> >>>>> >>> >>> > >
