cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jeff.Yu" <jeff...@iona.com>
Subject Re: [Proposal] Sample Improvement
Date Fri, 17 Aug 2007 05:18:58 GMT
Hi,

See my comment inline.

Thanks
Jeff


Glen Mazza wrote:
> OK, Round #2 Jeff--I yielded on 50% of the issues:
>
>
> Am Donnerstag, den 16.08.2007, 10:45 +0800 schrieb Jeff.Yu:
>   
>> Hi, Glen 
>>
>> Thanks for your comment, see my comments inline.
>>
>> Thanks
>> Jeff
>>
>>
>> Glen Mazza wrote:
>>     
>>> Hello Jeff,
>>>
>>> Here's my comments on each patch.
>>>
>>> Patch #901:
>>> 1.)  
>>>
>>> +    <target name="copy-war-libs" unless="without.libs">
>>> +    	<copy todir="${war-lib}">
>>> +    	   <fileset dir="${cxf.home}/modules">
>>> +					<include name="cxf-*.jar" />
>>> +    		</fileset>
>>> +    		<fileset dir="${cxf.home}/lib">
>>> +    		   <exclude name="servlet-api-*.jar" />
>>> +           <exclude name="geronimo-servlet_*.jar" />
>>> +           <exclude name="cxf-*.jar" />
>>> +    		</fileset>
>>> +    	</copy>	
>>>      </target>
>>>
>>> I think the listing of jar files needed should be explicitly listed via
>>> <includes/> (even if there are 40 of them--remember this is only done
>>> once in the common_build.xml), not *all* included but just having a few
>>> left out via <excludes/>.  This also will provide us a crisp,
>>> authoritative list of precisely what is needed to run CXF.  Using
>>> excludes over time might result in unneeded jars getting into the WAR
>>> files (i.e., we add a JAR file to the /lib directory but forget to
>>> exclude it here.)
>>>   
>>>       
>> Well, I don't think we need to list them, because in fact we include *all* the jars.
Allowing me explain a little bit on this.
>> Currently, we have two flavors CXF jar, one is all-in-one jar, called cxf-2.1-incubator.jar;
the other is module-based jar, which is listed in the "${cxf.home}/modules",
>> for the WARs, we used the module-based jars, so we need to exclude the all-in-one
jar.
>> The reason why I excluded those two servlet jars, because we deploy it into a servlet-container,
there is no need to have these two servlet jars. 
>>
>>     
>
> Hmmm, in the lib directory there is a "needed jars" file, which
> specifies a much smaller subset of all the jars in lib.
>
> The way I see it, is as follows, is you just need something like this:
> <war>
> ...
> <lib dir="${lib.dir}">
>    <include name="abc.jar"/>
>    <include name="bcd.jar"/>
>    ...
> </lib>
> ...
> </war>
>
> For the time being, best to just include the jars needed by *any* of the
> samples.  Later, perhaps, we can think of a cleaner design where we can
> bring in JARs required of *all* samples, plus a few extra
> sample-dependent jars, depending on the sample.
>
>
>   

The reason that why you think the "WHICH_JARS" in the lib lists much smaller jars, is it uses
the all-in-one jar as I said before. Well, I can update it to use the the all-in-one
jar instead of module-based jars for the simplicity, but I still think list 40 more jars is
not a good idea. If we want to make users clear which jars will be needed for their usage,

I think that is why the "WHICH_JARS" existed.

 

>>> 2.)  
>>>
>>> Deploy the application into APACHE TOMCAT with the commond:
>>> +  ant deploy-tomcat
>>>  
>>> Undeploy the application from the APACHE TOMCAT with the command:
>>> +  ant undeploy-tomcat
>>>
>>>
>>> Can the instructions on tomcat-deploy and tomcat-undeploy be moved to
>>> the common samples README without too much loss in user understanding,
>>> instead of being copied in every sample application's instructions?
>>> That would greatly simplify maintenance.  (BTW, it's nice that you got
>>> rid of that redundant -Dtomcat = true option)
>>>   
>>>       
>> I think it is better for us to keep it in those specific samples, first of all, not
all samples have this "running demo in a servlet container", only 7 or 8 samples,
>> and for these samples, since it has this specific section, it would be better for
us to keep it in this section.
>>
>>     
>
> It is not necessary for information in the common parent README.txt to
> be applicable for *every* sample--just as long as the text in the parent
> README indicates that.
>
> All you need to do is say in the "child" READMEs is something like:
> "This sample supports Tomcat deployment.  Please see the readme file in
> the sample root directory for more information."
>
> And in the parent README, something like:
>
> "Several of the samples support Tomcat deployment--please see the README
> file located with the sample to see if it is supported.  For those
> samples supporting Tomcat deployment, you can deploy the application
> with the following command:
>
>   
>>> +  ant deploy-tomcat
>>>  
>>>       
> and undeploy with this command:
>   
>>> +  ant undeploy-tomcat
>>>       
>
> Later, as we add more information to this, it will just need to go in
> this one common place.  And if we have to fix misspellings (see your
> "commond" above with ant deploy-tomcat, for example), we fix them in one
> place.
>
>
>   
Sounds good, will update the patch as you suggested.


>>> Patch #902: 
>>> 3.)  Is the idea of supporting Java-first web services *without* adding
>>> annotations[1] supported by the JAX-WS standard?  I believe this is what
>>> this sample is about, correct?  I'm not certain why we should be
>>> encouraging this style of coding--it doesn't seem very rigorous to
>>> program web services this way.
>>>
>>> This is mainly a philosophical concern, not a comment on the code as a
>>> whole.
>>>
>>> [1] http://cwiki.apache.org/CXF20DOC/simple-frontend.html
>>>   
>>>       
>> Yes, you are right, it doesn't have the annotation (JSR-181), it is another front-end
(POJO based) other than JAX-WS front-end. 
>> we are not encouraging it, but we need to have a sample for the completeness, we
will leave the decision to users.
>>
>>     
>
> OK.
>
>
>   
>>> 4.)  I would take a look at this patch (#902) again--are their any
>>> classes or interfaces being used which are *identical* to ones in other
>>> samples?  I don't know how others feel, but I would just reference them
>>> in the ant.build file when compiling and creating the WAR file, instead
>>> of constantly recreating identical classes.  This will also help us in
>>> the future when we starting factoring out common classes that are used
>>> by multiple samples.  
>>>
>>>   
>>>       
>> I don't like reusing too much, but I would like to hear others comments on this...
 
>>     
>
> OK, that's acceptable.
>
> Regards,
> Glen
>
>
>
>   

Mime
View raw message