Hi, Yeah, the changes are fine. I'll do them in a bit. Why must Joran actions have a logger per instance, out of curiosity?
Yoav Shapira http://www.yoavshapira.com --- Ceki G�lc� <[EMAIL PROTECTED]> wrote: > Yoav, > > Very nicely done. Thanks. > > Two small comments though. First, Actions MUST have a logger per > instance, not a static one. > > So, for example, > > if(getNamingContext() != null) { > LOGGER.warn("Overwriting existing naming context."); > } > > should be written as > > if(getNamingContext() != null) { > getLogger().warn("Overwriting existing naming context."); > } > > Second, you might consider taking advantage of the new parameterized > printing methods. So, instead of > > getLogger().warn("Missing " + JNDI_ATTR + " attribute, ignoring."); > > you can write > > getLogger().warn("Missing {} attribute, ignoring.", JNDI_ATTR); > > The latter form usually performs much better when the logging > statement is disabled. In this case, when statements of level WARN are > presumably enabled, it does not make big difference. > > If you agree with the above, I'll let you do the changes. > > More below. > > > At 10:08 PM 12/6/2004, Shapira, Yoav wrote: > > >Hi, > >After an incredibly long delay, I got around to implementing an initial > >version of this. I just committed it, along with a modified > >JoranConfigurator to use it. > > Better late than never. > > >I suppose the next step is to enhance the Joran unit tests to use this > >action. However, I wasn't sure what the best way is to modify the tests > >such that they use JNDI but don't break for everyone else ;) > > Yoav, do you have an idea how we could embed tomcat into our test > environment? It's such a sorely missing item at this stage.... > > > >Yoav Shapira http://www.yoavshapira.com > > -- > Ceki G�lc� > > The complete log4j manual: http://qos.ch/log4j/ > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
