logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ceki Gülcü <c...@qos.ch>
Subject RE: JNDISubstitution [was: LogLog Conversion: duplicate effort]
Date Tue, 07 Dec 2004 20:01:45 GMT

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:

>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: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org

View raw message