commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "James Carman" <ja...@carmanconsulting.com>
Subject RE: [proxy] Moving Proxy to Commons Proper
Date Wed, 19 Oct 2005 03:04:52 GMT
My comments are interleaved:

> i think that the proposal could be improved. it's usually used as the
> basis of the introduction paragraph for the component. a good proposal
> is a powerful weapon against featuritus and scope drift. so, it's
> important for the long term health of a project. 

> "The package shall create and maintain a suite of utility classes for
> creating proxy objects" seems just a little short. it seems to me that
> proxy is more like a minimal core of bridging API's for constructing
> proxy's (with minimal dependencies) supported by a number of optional
> implementations. 

I'm working on the proposal now.

> i would also like to reiterate stephen's warning: if you use interfaces,
> be very sure that you are not going to need to change them in any
> fashion. i would very strongly suggest implementing the key
> ProxyFactory logical interface as an abstract class. this isn't bias (i
> love interfaces when coding applications) but a hard lesson painfully
> learnt. 

I'm pretty sure about the ProxyFactory interface, actually.  I don't think
I'd actually be changing any of the existing method names/parameters.
However, I may have to add methods in the future.  That could cause problems
when folks upgrade, but I think I'd have the same problem if I used an
abstract class.  This is the first time that I've heard people tell me *not*
to use interfaces!  I understand the concern, though.  If anyone has any
suggestions for the ProxyFactory interface, I would be glad to hear them.
We really need to get this right.

> IMHO it'd be a good idea to note in the comment on the maven dependency
> page which dependencies are required for what code (preferably with an
> OPTIONAL at the start of the comment). a lot of users really complain
> (with good reason) about libraries with lots of core dependencies. the
> downside of maven (1) builds is that the list of dependencies can prove
> misleading.

Done, but I just appended "(optional)" rather than using a prefix because
Maven, for some reason, didn't include the comment when I prefixed it with
"OPTIONAL:".

> the maven mailing list page doesn't seem to have any information.

Fixed.

> you have some javadoc warnings and checkstyle violations.

Fixed.

> the distribution build doesn't load the notice and license into the jar
> MANEFEST. please read
> http://jakarta.apache.org/commons/releases/prepare.html for more
> details. 

Fixed.

> commons-proxy.imi, commons-proxy.ipr and lo4j.properties files need a
> license boilerplate.

I fixed log4j.properties, but every time I edit the Intellij IDEA files,
IDEA overwrites them.  Am I not supposed to be including IDEA project/module
files?  How about Eclipse .classpath and .project files?  What's our policy
on this?

> navigation.xml lists the project as beanutils

Fixed

> status.html still says 'collections' in a number of places. the list of 
> required runtime dependencies is probably now outdated.

Fixed

> ProxyUtils may well encounter classloading difficulties in some
> containers. it is possible that the context classloader may be null or
> inaccessible. i'm not convinced that the instantiateProxyFactory code
> will cope gracefully under all circumstances.
> http://jakarta.apache.org/commons/logging/tech.html may prove useful.

I'm going to have to look into this and maybe put in some test cases.

> on a slightly more serious note, the proposal says:

> "The initial codebase will be contributed by James Carman based on code
> in the Syringe (http://syringe.dev.java.net) project and can be
> distributed under the Apache license."

> has the incubator checked the paperwork for this?

Yes, the software grant paperwork has been filed.

> basic package level document. a lot of components include a basic
> introduction in the package level java docs. the information already
> contained in status and proposal document would make a good start.

Working on this still.

> since there are a lot of optional dependencies, it would be a very good
> idea to indicate in the javadocs which classes require which optional
> dependencies. for some packages, the package level documentation could
> be used for this.

This is done as far as I can tell.  I took care of most of the classes that
required outside dependencies.



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message