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 [EMAIL PROTECTED] 2005-05-30 17:40 ------- (In reply to comment #12) > 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? Arggh. Good catch. In this version handleFlawedDiscovery always throws a LogConfigurationException, but still discoverLogImplementation shouldn't count on that fact. > > 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. > +1. I don't like that stuff in here either, and your static initializer idea removes any remote justification for it. > 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. > Haven't thought carefully but this could perhaps be handled with a simple "if (logConstructor == null)" test around the code that sets instance variables. I too would prefer to see the isXXXAvailable methods go, even more so now that you've pointed out the can have side effects. > The comments on handleFlawedDiscovery still reference #FAILURE_PROPERTY, > though > I presume the actual code is in a different patch. Yes. Will fix. Thanks for the careful review. -- 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
