Simon Kitching wrote:
Hi,

Currently the Log4JLogger code in svn has this horrible stuff all
through it:
  if (is12) {
    ...
  } else {
    ...
  }

This is to handle the fact that log4j versions 1.2 and 1.3 are expected
to be binary incompatible in both directions, ie code compiled against
1.2 won't work against 1.3 and code compiled against 1.3 won't work
against 1.2.
The hack does allow code compiled against 1.3 to run against 1.3.
However there are a number of disadvantages:
* it requires compiling against 1.3 which isn't released yet and may be many months away.
* it has a performance hit
* it isn't very readable
* it isn't future proof; when log4j 1.3 comes out, this code
  won't work as the Priority class will go away completely.

I would therefore prefer to split this class into separate Log4J12Logger
and Log4J13Logger. A static initializer block in each will check which
version of log4j is available, and throw an exception to declare
themself "unavailable" if the wrong log4j version is present. Both
adapters can also be included in the list of discoverable logging
classes.

I think it would be a wise move to split the class in two. A while back I spent a fair amount of time trying to understand the differences betwen log4j 1.2 and 1.3 and also to get commons-logging to work with both log4j versions. It was confusing to say the least, and splitting this class would at least make the commons-logging code more understandable.

Of course removing a class will mean a 1.1 version number, but that's
good for a number of reasons.

The only other major concern I see is config files or system properties
that explicitly request the old logadapter by name. But that should be
easily fixable, and a config change seems a reasonable thing to do in
a .x release to me. Alternatively we could provide a trivial Log4JLogger
class that just extends Log4J12Logger; however I suspect this will cause
more pain and confusion than simply reporting a failure, esp. when log4j
1.3 is released and starts to become widely used.

I agree that, unless people have extended Log4JLogger, the only problems that could arise for users of commons-logging would be to change their config-files.

If we hold on to the Log4JLogger class for a while and let it extend Log4J12Logger, as you suggest, we might get away with doing a 1.0.X release.

I have a patch ready, but haven't committed it yet in order to give
y'all a chance to comment...

--
Dennis Lundberg

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to