On Nov 10, 2012, at 9:29 AM, Adrian Crum <adrian.c...@sandglass-software.com> 
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"/>
>>     -   <!-- 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.
> 
> 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
> 

Reply via email to