Anywhere within log4j we should think Appender, and forget about the existence of AppenderSkeleton. Changing the contract of the interface should be done with extreme caution.
However, if changing the contract is for good reasons, then I am in favour of it, as long as we provide good documentation about "Why my custom appender doesn't work in 1.3" in big bold flashing lights.
Paul
Curt Arnold wrote:
On Feb 25, 2005, at 3:22 PM, Ceki G�lc� wrote:
At 09:48 PM 2/25/2005, Curt Arnold wrote:
I can't stress how strongly that I believe these changes are bad and will unnecessarily cause some users of 1.2 to stay with log4j 1.2 or encounter broken behavior during the migration. I was disappointed that such significant changes were initially committed (back on the 18th) without any prior discussion or announcement. If I missed it, I apologize.
I understand. OK, to summarize:
1) The changes are compile-time backward compatible -- existing appenders will compile just fine without change against 1.2 as well as 1.3.
Existing appenders that derive from AppenderSkeleton will compile. Any, at this point hypothetical, appenders that directly implemented Appender without extending AppenderSkeleton will fail to compile.
2) The changes only affect programmatic calls to appenders maintained outside the log4j project. If those appenders are invoked from log4j configuration files, then they will work just fine and without change with 1.2 as well as 1.3.
I haven't confirmed it, but from code review it appears that any existing custom appended that implemented activateOptions will not be properly initialized from configuration files, since ConfigurationBase calls activate, not activateOptions. For a custom appender that required activateOptions and wanted to work with both log4j 1.2 and 1.3, it appears that the would have to implement both activateOptions and activate and avoid calling one from the other to avoid loops but avoid doing activation twice for one call. Let me know if you want be to build a test case to check this.
3) Applications such as hivemind which configure their custom appenders programmatically need to add one line to their code in order to invoke the activateOptions() method. If they do that, they will be compatible with both 1.2 and 1.3.
For Appenders that are ready at construction, they would more likely do what you did for tests/.../VectorAppender and just call super.activate() in the constructor.
By adding both AppenderSkeleton() and AppenderSkeleton(boolean), we allow existing custom to continue to work consistent with 1.2 and allow internal implementations to take advantage of the check that activate/activateOptions has been called. If we wanted to encourage custom appenders to migrate away from the compatibility behavior, then we could mark the default AppenderSkeleton constructor as deprecated.
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
