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" <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 >>> >>> > >
