Roland Berger wrote:
> Hi Phil
> 
> I have changed the naming.dtd to allow resourclink elements.
> The attached winrar compressed file contains all the changed and new 
> sources.

Looks good, though we need to talk about nested contexts.

> Please note the following:
> 
> - XMLConfigurator allows more than one context. This was done by 
> removing your ugly hack. After that all the XMLConfigurator test cases 
> were broken. After some hours I found out that the method 
> destroyInitialContext() did not work correctly when more than one 
> binding existed in the inital context.  See the comments in the method.
> The new implementation is still not the best. An iterator over a 
> collection, wich allows delete operation during iterating over the 
> collection would be nice. Obviously NamingEnumerator does not.

Also looks good on initial review. More tests would be good.
> 
> - The global naming context is indicated by an empty name attribut of 
> the context element. The global context is the initial context.

That is fine by me.
> 
> - The context used in ResourceLinkRef is initialized in 
> setupInitialContext(). I'm not sure if this a good solution since I 
> don't know the concepts of the SelectorContext class. Please check if 
> this soulution works also when the SelectorContext is used instead of 
> the NamingContext.

The SelectorContext is just a delegating wrapper for NamingContext that 
routes requests from bound threads / classloaders to their bound 
contexts.  This should be OK.

> 
> - By mistake I have reformated the source code. Sorry. Is there an 
> apache wide style guide or better a config file for eclipse an intellij?

As a general rule, it is best to follow the style of the surrounding 
code.  The bulk of the naming code came from tomcat, which more or less 
follows the Sun standard coding conventions.  The config package is a 
little different.  Don't worry about this issue specifically, but see 
important comment below.

> 
> - Let me know if you are going to commit the source or if my changes 
> does not meet the concepts you have defined for this project. If you'll 
> commit it I will keep my work in sync, otherwise I'll go my own way.

I would be happy to keep working with you on this, incorporating ideas 
and feedback from others as well. It will be easier for all involved if 
you submit patches instead of source archives.  Assuming you have a 
subversion client installed, you can get a patch by just navigating to 
the directory in your local checkout where the changed file(s) are 
located and typing

svn diff > name.patch

where name describes the patch.  Since the diff flags *all* lines that 
you have changed, its much better not reformat unchanged code as you 
make changes.  That makes it easier for all of us to see what has 
changed.  Its also best to open Jira tickets to track issues / 
improvements and to attach patches to them.

Here are some useful links on contributing
http://www.apache.org/dev/contributors.html
http://www.apache.org/dev/contributors.html#patches (though need the 
next one for how to work with svn)
http://svnbook.red-bean.com/en/1.1/svn-book.html

I will finish reviewing and commit most of your last submission as is, 
so don't worry about redoing that. Just please submit diffs in the future.

> 
> - On monday I'll implement a server.xml to naming.xml transformer. Are 
> you interested in this?

Most definitely.
> 
> - If I still have time I'll think of a solution for the nesting context 
> stuff.

That would be great.

Thanks!

Phil