geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jeremy Boynes" <jer...@coredevelopers.net>
Subject RE: [PATCH] intitial add DDBeanImpl and DDBeanRootImpl - first take
Date Thu, 21 Aug 2003 22:53:20 GMT
Chris

I have other issues as well.

* The dependency on Commons-Lang is just to provide HashCodeBuilder and
ToStringBuilder
  I am not convinced we need to introduce another library just to do this

* You use System.out in several places, presumably for debugging.
  Please use Commons-Logging at trace level for this

* You have a main method in these classes; this is not an entry point for
the
  system. Please use JUnit for testing

Finally, the impl of getChildNodes creates new DDBeanImpls() - should it be
doing this? I would have expected to need a factory for the specific
subclasses.

--
Jeremy

> -----Original Message-----
> From: Aaron Mulder [mailto:ammulder@alumni.princeton.edu]
> Sent: Tuesday, August 19, 2003 8:31 AM
> To: geronimo-dev@incubator.apache.org
> Subject: Re: [PATCH] intitial add DDBeanImpl and DDBeanRootImpl - first
> take
>
>
> On Mon, 18 Aug 2003, Chris Opacki wrote:
> > org.apache.geronimo.enterprise.deploy.tool.DDBeanImpl
> > org.apache.geronimo.enterprise.deploy.tool.DDBeanRootImpl
>
> Chris,
>
> 	It looks like there are a couple issues here:
>
>  - Commons Lang and Dom4j need to be added to the dependency list for
> these to compile
>
>  - They don't fully implement the JSR-88 interfaces that are currently in
> CVS.  I suspect you're using the 1.0 version of JSR-88, whereas the
> version in CVS includes new methods defined in the update for J2EE 1.4
> (v1.1):
>
> http://java.sun.com/j2ee/tools/deployment/88ChangeLog1.1_aug2802.html
>
>  - The main method has a reference to a specific file name
> (c:\java_specs\...)
>
> 	I'll clean this up a bit when I get a chance.
>
> Aaron
>
>


Mime
View raw message