commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 34661] - [logging][PATCH] Improvements to LogFactoryImpl
Date Mon, 30 May 2005 07:27:57 GMT
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=34661>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=34661





------- Additional Comments From skitching@apache.org  2005-05-30 09:27 -------
re patch 3a:

In discoverLogImplementation, when the user has specified an explicit log class
then we shouldn't do any discovery, yes? We should just load that logging class
or fail. But I'm not sure that is what the code is doing. If
handleFlawedDiscovery returns, then the flow drops down into the discovery part,
right? So maybe the code needs a throw new LogConfiguratioException after the
last handleFlawedDiscovery? And then the log4j discovery doesn't need to be in
an else, as that if-statement never exits out the bottom.

When testing for the jdk14 logger, I'm not sure that the code to check that java
1.4 is actually available belongs here. I'm surprised that it would be possible
to instantiate a Jdk14Logger in a pre-1.4 JVM at all, as the class does have
parameters with types only available in java1.4. However if java's "lazy
linking" is so advanced that it doesn't even resolve those types until needed,
then we can simply fix that by having a static block in Jdk14Logger that
references Level.class or similar. It seems to me to be cleaner for Logger
classes to be responsible for validating whether they can run or not than
putting this in the LogFactoryImpl class.

I think there's a copy-and-paste error in the comments for the Lumberjack test

Method createLogFromClass has side-effects: it sets a number of instance
variables as well as returning the Log object. But the isXXXAvailable methods
also call createLogFromClass - meaning that if anyone called one of those and it
succeeded, then this would "reset" the logConstructor and related members.
Possibly createLogFromClass could be split into two parts, with only the
side-effect-free part being called from the isXXXAvailable methods, but I would
prefer to simply do away with the isXXXAvailable methods completely.

The comments on handleFlawedDiscovery still reference #FAILURE_PROPERTY, though
I presume the actual code is in a different patch.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message