cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Glen Mazza <glen.ma...@verizon.net>
Subject Re: [Proposal] Sample Improvement
Date Thu, 16 Aug 2007 19:15:33 GMT
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.


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


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