= Changes To LoggerManager =


Anton Tagunov wrote:
What would you say to the following changes to
*LoggerManager:

1) The creator of LoggerManager passes an initial
   logger to it.

yeppo! I am wondering though how much logging the LoggerManager actually needs to do before initialization. And hence wondering what would be actually logged to this initial logger. Keep in mind that the client of LoggerManager should not ask it to do anything until it is initialize()d (per avalon-framework contracts). So the only messaging left is that from the LoggerManager itself.


And that is normally served best by implementing LogEnabled.

comments?

2)     XXXLoggerManager(
               String prefix,
               Logger initialLogger,
               String switchToCategory )

I guess the same sidenote applies here :D


3) One more thing I'm going to do [LogKitLoggerManager]

+/-0 (don't use it; dunno)


4) m_hierarchy.getRootLogger().unsetLogTargets()

+/-0 (don't use it; dunno)


5) But if the hierarchy has been passed to us already created,
   should we do _any_ of this?

I think: not. The general assumption I guess is that the entity (usually a piece of a container) passing in the hierarchy has intimate knowledge of what should happen.


6)
   There's an option to test if our logging is alive
   in the end of configure() method by doing

getLoggerForCategory("").info("Logging started");

Please, give me an advice on this feature.

not knowing the code well, this seems like a weird way to test if you can log. Don't know logkit well, but I assume it has a "logEnabled" method or two.


However, why would logging not be started? Configuration problem? On the basis of deterministic configurations you should be able to predict that by checking the configuration validity. If you do that and put in place reasonable unit tests then you can trust there is no error.

Going even further, I think anything that implements Logger should be very tolerant of configuration validity (falling back to defaults or complaining in the logs rather than throwing exceptions) because logging is a facility which should simply always work.

hmm. My lack of knowlegde about logkit shines through :D

7) LogKitLoggerManager(
           String prefix,
           Logger defaultLogger,
           Logger interanlLogger )
   {
       m_defaultLogger = defaultLogger;
       ...
   }

   Logger getDefaultLogger()
   {
       return m_defaultLogger;
   }

I must confess I sort of dislike this.

hmm, to me it sounds rather sensible & predicatble :D


   With regards to this issue I would consider the following
   options:

a) remove this constructor completely

can't do that. This is released code, after all!


   b) deprecate this constructor but retain functionality
   c) leave the constructor undeprecated

   I would like an advice on this, I just do not dare to
   do a) and b) without a permit, although I feel very much
   inclined to do one of this.

perhaps you can explain a little more just why this constructur sucks so bad....I can imagine valid use cases. Furthermore, I can imagine it actually being used in places.


8) LogKitLoggerManager(
           String prefix,
           Logger defaultLogger)

   in line with changes discussed in section 7) I would
   like to change the contract

again, changing contracts on released code is something I'm weary of. What use cases would you satisfy like this and what would you break?



= Talk First or Code First? =


I would certainly not like to make drastic changes untill
allowed to. On the other hand maybe it's better to just
go ahead and implement it and _then_ have a discussion
on why I did that and probably revert some of the changes?

that depends on how sure you are of the changes you want to make. If there is a reasonable chance your changes might affect people in a bad way or will not be accepted because you suspect there might be a better way to do things, it's better to talk first, commit later. When that chance is rather slim, commit first, talk if there is a problem, saves time on everyone.


Of course, being a new guy here, it might be a little difficult for you to assert what is a "reasonable chance", and staying on the safe side (like you're doing) is a good idea :D. IMO, stay on the safe side 'till you no longer have to actually think about these assertions!

In general, you should never modify the public API of released code in a potentially incompatible way without discussing it first, and be very careful with deprecation as well. Of course, some APIs are more public than others: excalibur-logger is mostly container-internal, so a truly general rule does not apply. Common sense does!

cheers!

- LSD



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



Reply via email to