Hi Wolfram,
thanks for making the changes!
Your patch looks just perfect!
I was able to apply your diffs just fine.
I have sent the DOL related changes to the deployment lead (Hong
Zhang), so she gets a chance to review them also before I commit them
on your behalf.
Some very minor things I noticed that you might want to keep in mind
for any future patch submissions: :)
- Use spaces instead of TABs
- Avoid lines longer than 80 characters (see Java Coding Conventions)
I went ahead and made these "cosmetic" adjustments in the very few
places in your diffs where they were needed.
Also, I added a "return" statement here (in WebModule.java):
+ if (!(valve instanceof GlassFishValve) &&
+ !(valve instanceof Valve)) {
+ logger.log(Level.WARNING,
+ "Object of type classname " + className +
+ " not an instance of Valve or GlassFishValve");
+ }
I also went ahead and added a message key for the above warning
message, so it can be localized (I made the same change in the other
place in
the existing code that had the same issue).
That's it!
Once I hear back from Hong, I will commit your changes on your behalf.
You mentioned you also have a test case for this feature. Feel free to
send it my way, so I can integrate it as well.
Once your changes have been committed, we'll work with June Parks
(cc'ed), so she can update the Developer Guide and also take care of
deprecating the property based approach.
Thanks again for your first patch and contribution, Wolfram!
Excellent work!!
Jan
On 02/12/09 13:45, Wolfram Rittmeyer wrote:
> Hi,
>
> I have added the patches for the web and deployment projects as
> diff-files. Also I have added the two new classes
> com.sun.enterprise.deployment.node.runtime.web.ValveNode and
> com.sun.enterprise.deployment.runtime.web.Valve that belong into the
> dol project of the deployment projects.
>
> Due to the new DTD (sun-web-app_3_0-0.dtd) some minor changes had to
> be added to the DTDRegistry (dol project) and WebBundleRuntimeNode.
> These changes have not been part of my first patch.
>
> Jan, you mentioned in one of your earlier mails that I should
> deprecate property-based valve handling. This actually only applied to
> the addValve(java.lang.String) method, if I am not mistaken. I did add
> a deprecation-annotation and comment but I guess more relevant would
> be the documentation which momentarily mentions the property usage.
>
>
> Thanks for your help and comments to my first patch,
>
> --
> Wolfram Rittmeyer
>
> ------------------------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: webtier-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: webtier-help_at_glassfish.dev.java.net