On 1/08/2010, at 11:24 AM, Adrian Crum wrote:

> --- On Sat, 7/31/10, Scott Gray <scott.g...@hotwaxmedia.com> wrote:
>> Yeah whoops, thanks for fixing that
>> up.
>> 
>> Inline
>> 
>> Regards
>> Scott
>> 
>> On 1/08/2010, at 3:55 AM, Adrian Crum wrote:
>> 
>>> Scott,
>>> 
>>> I fixed the code and built it out a little.
>>> 
>>> This is a good idea, but I have a couple of problems
>> with it:
>>> 
>>> 1. User-oriented meta-data should be kept in
>> properties files - so it can be internationalized. The
>> drawback to that is, developers will be less likely to
>> include descriptions if they have to do it in a separate
>> file.
>> 
>> It wasn't my intention for the meta-data to be oriented
>> towards the user, I see it as developer documentation
>> only.  A few times in the past I've noticed it can be
>> difficult to describe an attributes purpose from the service
>> description and inlining them was intended to make it a
>> little more like a javadoc.
> 
> By user I meant developer. If it's developer information only, then why store 
> it in the model? A developer could just look at the service definition file.

I agree it doesn't necessarily need to be in the model, webtools could just as 
easily go and look at the service def itself.

>>> 2. Description text should not be stored in the model
>> - it increases the model's memory footprint greatly.
>> Instead, the descriptions should be retrieved from
>> properties files when needed (in AvailableServices.groovy,
>> for example).
>> 
>> Well I agree with you except for the properties file part,
>> but I guess that comes back to #1 above.  We could
>> always look at doing some sort of lazy loading, where the
>> descriptions are only added to the model if they're
>> requested.  Considering that viewing the service in
>> webtools is the only way to expose the descriptions, you'd
>> probably never end up with more than a few dozen actually
>> loaded into memory.
> 
> The reason I suggested the properties file is because this issue came up 
> before with the entity field descriptions. At the time those were being 
> added, there was interest in internationalizing them.

Maybe we should internationalize java comments as well?  Just because people 
are interested in something doesn't make it a good idea.

> From my perspective, using a properties file is easier than the existing 
> implementation. In the AvailableServices.groovy script, concatenate the 
> service name and attribute name, then use that as a key to look up the 
> description in a properties file. No need to add anything to the model.

I don't think we are so good at documentation that we should be okay with 
making it any harder :-)

>>> 
>>> -Adrian
>>> 
>>> 
>>> --- On Sat, 7/31/10, Adrian Crum <adrian.c...@yahoo.com>
>> wrote:
>>>> Scott,
>>>> 
>>>> Are you sure this does what you intended? The
>> parameters
>>>> are being assigned the service's description, not
>> the
>>>> parameter's description.
>>>> 
>>>> -Adrian
>>>> 
>>>> --- On Sat, 7/31/10, lekt...@apache.org
>>>> <lekt...@apache.org>
>>>> wrote:
>>>> 
>>>>> From: lekt...@apache.org
>>>> <lekt...@apache.org>
>>>>> Subject: svn commit: r981018 - in
>>>> /ofbiz/trunk/framework/service: dtd/services.xsd
>>>> src/org/ofbiz/service/ModelParam.java
>>>> src/org/ofbiz/service/ModelServiceReader.java
>>>>> To: comm...@ofbiz.apache.org
>>>>> Date: Saturday, July 31, 2010, 1:26 AM
>>>>> Author: lektran
>>>>> Date: Sat Jul 31 08:26:42 2010
>>>>> New Revision: 981018
>>>>> 
>>>>> URL: http://svn.apache.org/viewvc?rev=981018&view=rev
>>>>> Log:
>>>>> Add a description element for service
>> attributes,
>>>> should
>>>>> make documenting some services a little
>> easier. 
>>>> Still
>>>>> need to display it in webtools somewhere.
>>>>> 
>>>>> Modified:
>>>>>     
>>>>> 
>> ofbiz/trunk/framework/service/dtd/services.xsd
>>>>>     
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>>>>     
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>>>> 
>>>>> Modified:
>>>> ofbiz/trunk/framework/service/dtd/services.xsd
>>>>> URL: 
>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/dtd/services.xsd?rev=981018&r1=981017&r2=981018&view=diff
>>>>> 
>>>> 
>> ==============================================================================
>>>>> ---
>> ofbiz/trunk/framework/service/dtd/services.xsd
>>>>> (original)
>>>>> +++
>> ofbiz/trunk/framework/service/dtd/services.xsd Sat
>>>> Jul
>>>>> 31 08:26:42 2010
>>>>> @@ -314,6 +314,7 @@ under the License.
>>>>>       
>>>>>     <xs:complexType>
>>>>>           
>>>>>     <xs:sequence>
>>>>>            
>>   
>>>>>     <xs:element minOccurs="0"
>>>>> maxOccurs="unbounded"
>> ref="type-validate"/>
>>>>> +           
>>    
>>>>> <xs:element minOccurs="0" ref="description"
>> />
>>>>>           
>>>>>     </xs:sequence>
>>>>>           
>>>>>     <xs:attributeGroup
>>>>> ref="attlist.attribute"/>
> 
>>>>>       
>>>>>     </xs:complexType>
>>>>> 
>>>>> Modified:
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>>>> URL: 
>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java?rev=981018&r1=981017&r2=981018&view=diff
>>>>> 
>>>> 
>> ==============================================================================
>>>>> ---
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>>>> (original)
>>>>> +++
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java
>>>>> Sat Jul 31 08:26:42 2010
>>>>> @@ -43,6 +43,9 @@ public class ModelParam
>> implements
>>>> Seria
>>>>>       /** Parameter name */
>>>>>       public String name;
>>>>>    
>>>>> +    /** The description of this
>> parameter */
>>>>> +    public String description;
>>>>> +
>>>>>       /** Paramater type */
>>>>>       public String type;
>>>>>    
>>>>> @@ -88,6 +91,7 @@ public class ModelParam
>> implements
>>>> Seria
>>>>>    
>>>>>       public
>> ModelParam(ModelParam
>>>>> param) {
>>>>>           this.name =
>>>>> param.name;
>>>>> +        this.description
>> =
>>>>> param.description;
>>>>>           this.type =
>>>>> param.type;
>>>>>           this.mode =
>>>>> param.mode;
>>>>>          
>> this.formLabel =
>>>>> param.formLabel;
>>>>> 
>>>>> Modified:
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>>>> URL: 
>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java?rev=981018&r1=981017&r2=981018&view=diff
>>>>> 
>>>> 
>> ==============================================================================
>>>>> ---
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>>>> (original)
>>>>> +++
>>>>> 
>>>> 
>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelServiceReader.java
>>>>> Sat Jul 31 08:26:42 2010
>>>>> @@ -518,6 +518,7 @@ public class
>> ModelServiceReader
>>>>> implemen
>>>>>           
>>>>>     ModelParam param = new
>> ModelParam();
>>>>>    
>>>>>           
>>>>>     param.name =
>>>>> 
>>>> 
>> UtilXml.checkEmpty(attribute.getAttribute("name")).intern();
>>>>> +       
>>    
>>>>> param.description = getCDATADef(baseElement,
>>>>> "description");
>>>>>           
>>>>>     param.type =
>>>>> 
>>>> 
>> UtilXml.checkEmpty(attribute.getAttribute("type")).intern();
>>>>>           
>>>>>     param.mode =
>>>>> 
>>>> 
>> UtilXml.checkEmpty(attribute.getAttribute("mode")).intern();
>>>>>           
>>>>>     param.entityName =
>>>>> 
>>>> 
>> UtilXml.checkEmpty(attribute.getAttribute("entity-name")).intern();
>>>>> 
>>>>> 
>>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>> 
>>> 
>>> 
>> 
>> 
> 
> 
> 

Attachment: smime.p7s
Description: S/MIME cryptographic signature

Reply via email to