commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kris Nuttycombe" <Kris.Nuttyco...@noaa.gov>
Subject [pipeline] Logging style
Date Thu, 28 Jul 2005 15:45:28 GMT
Hi, Simon,

Thanks for taking the time to make this patch work. In the future I'll
make certain to test the patches in development environments other than
my own, to make certain that they run properly.

>Generally, @author tags are discouraged in commons components. They are regarded
>as not encouraging the community spirit and shared maintenance approach that
>commons hopes to occur for its projects. And over a projects' lifetime it is
>hoped the code will include many authors, which gets pretty clumsy if they all
>get @author lines. Instead, credit is given in the project.xml file (developer,
>contributor sections), and for particular cases the release  notes etc. And of
>course it's evident in the svn commit info in the usual case of the author
>committing their own changes. So while I have left files not touched by this
>patch set alone, I have removed @author tags where the patch was trying to
>update them. I hope this is ok with you.
>  
>
That's fine, and I will submit another patch to remove the rest of the
@author tags and the $Log$ information. I wasn't aware of the fact that
these are discouraged in commons, so thanks for letting me know.

>On a separate note, I have noticed that LogStage has this:
>    private Log log = LogFactory.getLog(this.getClass());
>while most other classes have this:
>    private static final Log log = 
>      LogFactory.getLog(FtpFileDownloadStage.class);
>
>The latter is more efficient in time and space as it only retrieves the logger
>once, rather than once per object. However if this code is ever deployed into a
>container's shared classpath then the static version is certainly wrong. Whether
>the per-instance version is right or wrong depends upon whether the object
>instance could be visible to multiple components (webapps/ejbs/etc).
>
>Just food for thought..
>  
>
Thanks for pointing that out; it hadn't occurred to me because we
haven't deployed the pipeline in a shared classloader.
LogFactory.getLog(this.getClass()) can likely also cause problems when
the class is extended by giving a misleading source for the log message,
so I'll change these to:

private final Log log = LogFactory.getLog(WhateverClassName.class);

>Anyway, pipeline now compiles and the tests all run clean.
>
Thanks again for your help,

Kris

---------------------------------------------------------------------
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