You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by robert burrell donkin <rd...@apache.org> on 2006/04/26 22:34:08 UTC
[VOTE] Release RC10 As Commons Logging 1.1
after some investigation, the issue which turned up with RC8 was
determined not to be a bug. notes have been added to the troubleshooting
documentation.
RC10 is available from:
http://people.apache.org/~rdonkin/commons-logging. please download,
check and then vote.
i will tally the votes no earlier than 2300GMT Wednesday 3rd Mat 2006
here's my +1
- robert
-- 8<
-----------------------------------------------------------------------------------------------------
[ ] +1 Release Candidate 10 As JCL 1.1
[ ] +0 Positive but have not had time to check release
[ ] -0 Negative
[ ] -1 Do not release Candidate 10 AS JCL
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by Dennis Lundberg <de...@apache.org>.
robert burrell donkin wrote:
> after some investigation, the issue which turned up with RC8 was
> determined not to be a bug. notes have been added to the troubleshooting
> documentation.
>
> RC10 is available from:
> http://people.apache.org/~rdonkin/commons-logging. please download,
> check and then vote.
>
> i will tally the votes no earlier than 2300GMT Wednesday 3rd Mat 2006
>
> here's my +1
>
> - robert
>
> -- 8<
> -----------------------------------------------------------------------------------------------------
> [X] +1 Release Candidate 10 As JCL 1.1
> [ ] +0 Positive but have not had time to check release
> [ ] -0 Negative
> [ ] -1 Do not release Candidate 10 AS JCL
--
Dennis Lundberg
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by Stephen Colebourne <sc...@btopenworld.com>.
robert burrell donkin wrote:
>>Maven does them as a source jar - anyone know if there's a reason for
>>the jar rather than zip extension in some of the iDEs?
>>
>>It also has a javadoc jar for IDEs.
>
> the jar's all-in-one (combined source+javadocs)
For reference, the recent [io] release has used -src-ide.zip as a
suffix, but I understand why you chose to drop the -src (because you've
included javadocs)
Stephen
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Thu, 2006-04-27 at 11:59 -0700, Henri Yandell wrote:
> On 4/27/06, robert burrell donkin <ro...@blueyonder.co.uk> wrote:
> > On Thu, 2006-04-27 at 14:06 +0100, sebb wrote:
> > > On 27/04/06, Simon Kitching <sk...@apache.org> wrote:
> > > > On Wed, 2006-04-26 at 21:34 +0100, robert burrell donkin wrote:
> > > > > after some investigation, the issue which turned up with RC8 was
> > > > > determined not to be a bug. notes have been added to the troubleshooting
> > > > > documentation.
> > > > >
> > > > > RC10 is available from:
> > > > > http://people.apache.org/~rdonkin/commons-logging. please download,
> > > > > check and then vote.
> > >
> > > What's the file
> > > commons-logging-1.1-RC10-ide.zip in
> > > commons-logging-1.1-RC10.zip for?
> >
> > for IDEs that support a source zip for libraries
>
> Maven does them as a source jar - anyone know if there's a reason for
> the jar rather than zip extension in some of the iDEs?
>
> It also has a javadoc jar for IDEs.
the jar's all-in-one (combined source+javadocs)
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by Henri Yandell <fl...@gmail.com>.
On 4/27/06, robert burrell donkin <ro...@blueyonder.co.uk> wrote:
> On Thu, 2006-04-27 at 14:06 +0100, sebb wrote:
> > On 27/04/06, Simon Kitching <sk...@apache.org> wrote:
> > > On Wed, 2006-04-26 at 21:34 +0100, robert burrell donkin wrote:
> > > > after some investigation, the issue which turned up with RC8 was
> > > > determined not to be a bug. notes have been added to the troubleshooting
> > > > documentation.
> > > >
> > > > RC10 is available from:
> > > > http://people.apache.org/~rdonkin/commons-logging. please download,
> > > > check and then vote.
> >
> > What's the file
> > commons-logging-1.1-RC10-ide.zip in
> > commons-logging-1.1-RC10.zip for?
>
> for IDEs that support a source zip for libraries
Maven does them as a source jar - anyone know if there's a reason for
the jar rather than zip extension in some of the iDEs?
It also has a javadoc jar for IDEs.
> > The files in it don't have the leading directory name which is present
> > in the top-level zips.
>
> yep
>
> AIUI that's the format they require
Yeah, same understanding here.
Hen
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Thu, 2006-04-27 at 14:06 +0100, sebb wrote:
> On 27/04/06, Simon Kitching <sk...@apache.org> wrote:
> > On Wed, 2006-04-26 at 21:34 +0100, robert burrell donkin wrote:
> > > after some investigation, the issue which turned up with RC8 was
> > > determined not to be a bug. notes have been added to the troubleshooting
> > > documentation.
> > >
> > > RC10 is available from:
> > > http://people.apache.org/~rdonkin/commons-logging. please download,
> > > check and then vote.
>
> What's the file
> commons-logging-1.1-RC10-ide.zip in
> commons-logging-1.1-RC10.zip for?
for IDEs that support a source zip for libraries
> The files in it don't have the leading directory name which is present
> in the top-level zips.
yep
AIUI that's the format they require
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by sebb <se...@gmail.com>.
On 27/04/06, Simon Kitching <sk...@apache.org> wrote:
> On Wed, 2006-04-26 at 21:34 +0100, robert burrell donkin wrote:
> > after some investigation, the issue which turned up with RC8 was
> > determined not to be a bug. notes have been added to the troubleshooting
> > documentation.
> >
> > RC10 is available from:
> > http://people.apache.org/~rdonkin/commons-logging. please download,
> > check and then vote.
What's the file
commons-logging-1.1-RC10-ide.zip in
commons-logging-1.1-RC10.zip for?
The files in it don't have the leading directory name which is present
in the top-level zips.
> >
> > i will tally the votes no earlier than 2300GMT Wednesday 3rd Mat 2006
> >
>
> > [X] +1 Release Candidate 10 As JCL 1.1
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by Torsten Curdt <tc...@apache.org>.
> > RC10 is available from:
> > http://people.apache.org/~rdonkin/commons-logging. please download,
> > check and then vote.
> >
> > i will tally the votes no earlier than 2300GMT Wednesday 3rd Mat 2006
> >
>
> > [X] +1 Release Candidate 10 As JCL 1.1
Let's get it out!
cheers
--
Torsten
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by Simon Kitching <sk...@apache.org>.
On Wed, 2006-04-26 at 21:34 +0100, robert burrell donkin wrote:
> after some investigation, the issue which turned up with RC8 was
> determined not to be a bug. notes have been added to the troubleshooting
> documentation.
>
> RC10 is available from:
> http://people.apache.org/~rdonkin/commons-logging. please download,
> check and then vote.
>
> i will tally the votes no earlier than 2300GMT Wednesday 3rd Mat 2006
>
> [X] +1 Release Candidate 10 As JCL 1.1
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by Dennis Lundberg <de...@apache.org>.
Simon Kitching wrote:
> On Wed, 2006-05-03 at 22:08 +0100, robert burrell donkin wrote:
>> please check my commits
>
> All looks good to me.
To me too.
You have covered all that was left on my to do list.
--
Dennis Lundberg
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by Simon Kitching <sk...@apache.org>.
On Wed, 2006-05-03 at 22:08 +0100, robert burrell donkin wrote:
> please check my commits
All looks good to me.
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Wed, 2006-05-03 at 22:32 +0200, Dennis Lundberg wrote:
> robert burrell donkin wrote:
> > On Tue, 2006-05-02 at 22:27 +1200, Simon Kitching wrote:
> >> Hi,
> >>
> >> To save people reading through below, my summary is:
> >>
> >> <summary>
> >> Some excellent proofreading here, and a number of doc/release-notes
> >> issues have been found by Dennis, as well as a case where we could use
> >> constants instead of inline strings, and one where we should technically
> >> be using a param value instead of a constant in a diagnostic message
> >> (though it doesn't matter in that case anyway).
> >>
> >> However I don't see anything here that I think is worth cancelling the
> >> RC10 vote for. A few items would be good to put up on the wiki under
> >> "1.1 release addendum".
> >
> > an interesting one, this
> >
> > the release process we use here in the commons (release candidates
> > rather than blessing a concrete distribution) means that there are
> > always changes between the final release candidate and the release
> > distributed. the question is what changes are acceptable and which
> > necessitate another VOTE.
> >
> > we already have changes to the documentation and some to the code
> > formatting committed. more changes (as outlined by dennis) shouldn't
> > really effect the result: either no changes above the minimum version
> > changes are acceptable or cosmetic and documentation ones are.
> >
> > it feels like a long, long we've travelled. after all this effort, i
> > think one final push is worth it. given the fact that the changes are
> > cosmetic and documentation, i think i'll cut one more candidate tomorrow
> > but propose a short length for the vote.
>
> +1
>
> None of the changes that I have committed affect the actual running code
> of JCL. The changes made are:
> - Corrections and additions to JavaDoc
> - Corrections to documentation (xdocs)
> - Code formating regarding white space (transforming tabs into spaces)
yep: it's more of a procedural issue. if we hadn't been working on this
release for so long i'd probably been happy just to tally the votes...
i have some more troubleshooting documentation which is yet to be tidied
up and committed. i'd be grateful if you could find the time to check
careful any changes i make.
> I deliberately did not check in the proposed *code* changes in
> LogFactory and LogFactoryImpl. As Simon correctly stated they really
> don't bring any added value. They are more design issues.
yeh - i appreciate that :)
> Also I have not committed the proposed changes/additions to the
> RELEASE_NOTES.txt. If there will be another RC then these changes should
> be committed.
> - Upgrade recommendations for the api jar (Tomcat vs all others)
> - The api jar still contains Jdk14Logger
> - AvalonLogger no longer implements serializable
please check my commits
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by Dennis Lundberg <de...@apache.org>.
robert burrell donkin wrote:
> On Tue, 2006-05-02 at 22:27 +1200, Simon Kitching wrote:
>> Hi,
>>
>> To save people reading through below, my summary is:
>>
>> <summary>
>> Some excellent proofreading here, and a number of doc/release-notes
>> issues have been found by Dennis, as well as a case where we could use
>> constants instead of inline strings, and one where we should technically
>> be using a param value instead of a constant in a diagnostic message
>> (though it doesn't matter in that case anyway).
>>
>> However I don't see anything here that I think is worth cancelling the
>> RC10 vote for. A few items would be good to put up on the wiki under
>> "1.1 release addendum".
>
> an interesting one, this
>
> the release process we use here in the commons (release candidates
> rather than blessing a concrete distribution) means that there are
> always changes between the final release candidate and the release
> distributed. the question is what changes are acceptable and which
> necessitate another VOTE.
>
> we already have changes to the documentation and some to the code
> formatting committed. more changes (as outlined by dennis) shouldn't
> really effect the result: either no changes above the minimum version
> changes are acceptable or cosmetic and documentation ones are.
>
> it feels like a long, long we've travelled. after all this effort, i
> think one final push is worth it. given the fact that the changes are
> cosmetic and documentation, i think i'll cut one more candidate tomorrow
> but propose a short length for the vote.
+1
None of the changes that I have committed affect the actual running code
of JCL. The changes made are:
- Corrections and additions to JavaDoc
- Corrections to documentation (xdocs)
- Code formating regarding white space (transforming tabs into spaces)
I deliberately did not check in the proposed *code* changes in
LogFactory and LogFactoryImpl. As Simon correctly stated they really
don't bring any added value. They are more design issues.
Also I have not committed the proposed changes/additions to the
RELEASE_NOTES.txt. If there will be another RC then these changes should
be committed.
- Upgrade recommendations for the api jar (Tomcat vs all others)
- The api jar still contains Jdk14Logger
- AvalonLogger no longer implements serializable
--
Dennis Lundberg
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Tue, 2006-05-02 at 22:27 +1200, Simon Kitching wrote:
> Hi,
>
> To save people reading through below, my summary is:
>
> <summary>
> Some excellent proofreading here, and a number of doc/release-notes
> issues have been found by Dennis, as well as a case where we could use
> constants instead of inline strings, and one where we should technically
> be using a param value instead of a constant in a diagnostic message
> (though it doesn't matter in that case anyway).
>
> However I don't see anything here that I think is worth cancelling the
> RC10 vote for. A few items would be good to put up on the wiki under
> "1.1 release addendum".
an interesting one, this
the release process we use here in the commons (release candidates
rather than blessing a concrete distribution) means that there are
always changes between the final release candidate and the release
distributed. the question is what changes are acceptable and which
necessitate another VOTE.
we already have changes to the documentation and some to the code
formatting committed. more changes (as outlined by dennis) shouldn't
really effect the result: either no changes above the minimum version
changes are acceptable or cosmetic and documentation ones are.
it feels like a long, long we've travelled. after all this effort, i
think one final push is worth it. given the fact that the changes are
cosmetic and documentation, i think i'll cut one more candidate tomorrow
but propose a short length for the vote.
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by Simon Kitching <sk...@apache.org>.
Hi,
To save people reading through below, my summary is:
<summary>
Some excellent proofreading here, and a number of doc/release-notes
issues have been found by Dennis, as well as a case where we could use
constants instead of inline strings, and one where we should technically
be using a param value instead of a constant in a diagnostic message
(though it doesn't matter in that case anyway).
However I don't see anything here that I think is worth cancelling the
RC10 vote for. A few items would be good to put up on the wiki under
"1.1 release addendum".
</summary>
On Tue, 2006-05-02 at 00:20 +0200, Dennis Lundberg wrote:
> robert burrell donkin wrote:
> > after some investigation, the issue which turned up with RC8 was
> > determined not to be a bug. notes have been added to the troubleshooting
> > documentation.
> >
> > RC10 is available from:
> > http://people.apache.org/~rdonkin/commons-logging. please download,
> > check and then vote.
>
> <snip>
>
> Sorry to be jumping into this at the last minute. I've done a code
> review of /src/java and /xdocs and compared them to the 1.0.4 release.
> As you might have seen I committed some spelling corrections to the
> xdocs. The added documents are excellent by the way!
Thanks for the doc patches. It's amazing how mistakes creep in!
>
> There were a couple of questions that popped up along the way, which I
> will deal with below, one file at a time. Nothing big, mostly
> documentation stuff, so nothing to hold up a release.
>
> The attached patch file fixes a couple of them, while others need input
> from you. I didn't want to commit the patch before I had checked it with
> the rest of you.
>
> I hope to be doing some practical tests in applet environments tomorrow.
>
>
> \commons-logging-1.1-RC10-src\RELEASE-NOTES.txt
>
> Lines 147-150:
> "Previous releases of commons-logging-api.jar contained the Jdk14Logger
> class;
> this is now deprecated. It will be removed from the API jar in some future
> release. If your application needs this jar, then instead of
> upgrading to commons-logging-api-1.1.jar, upgrade to
> commons-logging-1.1.jar."
>
> I don't see why we advice this. The jar still contains Jdk14Logger.
> Should we encourage people to switch jars now or when JCL 2 comes out?
We did want to remove Jdk14Logger from the api jar because it just
doesn't belong in an "api" jar.
However it later became evident that using a full JCL jar doesn't work
with Tomcat because tomcat loads JCL via the system classloader, and
also prevents any class loaded via that classloader from being
overridden by a webapp. We could provide another jar for this sort of
situation (commons-logging-tomcat.jar or similar name), but it's
probably more sensible to just live with a weird "api" jar that contains
Jdk14Logger until the 2.x series (as you say).
Unfortunately it looks like the release-notes weren't updated (probably
my fault). So that advice is actually wrong when applied to tomcat, but
good in other cases.
I think we could just make a note in the wiki about this misleading info
in the release notes. There is already some info on this tomcat issue
elsewhere in the JCL docs.
> Lines 173-174:
> "File commons-logging-api-nn.jar provides no adapters to external logging
> libraries, just the internally implemented SimpleLog and NoOpLog classes."
>
> This is not entirely true. The Jdk14Logger is there as well.
Yep, due to the reversal during the RC series where we agreed to
reinstate that class. The release notes are wrong, but it's not too
serious. Actually, if you take literally the statement "adapters to
external logging libraries", java.util.logging may not qualify :-).
I suggest a wiki note could again cover this.
> There is nothing in the release notes about the fact that AvalonLogger
> no longer implements Serializable. Do we need that?
Yes, the release notes probably should have pointed that out. The code
change is right; AvalonLogger never did serialize properly. In fact,
it's actually impossible given the design of AvalonLogger. As it never
worked, we can't be breaking any users by removing the feature.
This missing info in the release notes is not worth holding a release
back I think, esp. as it's obvious no-one ever tried to serialize an
AvalonLogger object.
>
>
> \commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\impl\AvalonLogger.java
>
> Line 251: Javadoc for trace(Object message, Throwable t) was changed
> like this:
> @see org.apache.commons.logging.Log#trace(java.lang.Object,
> java.lang.Throwable) ---> @see
> org.apache.commons.logging.Log#debug(Object, Throwable)
> Should be changed back.
> (done in patch)
Agreed. But not worth holding back a release for IMO.
>
>
> \commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\impl\Jdk13LumberjackLogger.java
>
> Line 42: A since-tag of "1.1" was added, but the class existed in SCM
> when the previous version was released, but was not included in the jar.
> Should the since tag be there?
I think that the since tag should indicate what *release* it was in, ie
that "1.1" is the right thing.
>
>
> \commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\impl\LogFactoryImpl.java
>
> Line 121: The sentence seems to not have been finished...
Yep. Should be something like:
When set to false, a LogConfigurationException is thrown if
LogFactoryImpl is loaded via a child classloader of the TCCL (this
should never happen in sane systems).
> Lines 173-175: Could use the constants that were defined in lines 74-78.
Yep, agreed (the constants were added later, to support diagnostic
output).
> Line 761,1322: Missing indentation
> (done in patch)
Actually, that was deliberate. I prefer that layout, where the throws
clause is not indented:
public void foo(.......)
throws BarException {
// first line
}
However I can live with the (more common) proposed format:
public void foo(.......)
throws BarException {
// first line
}
> Line 1252: Should be removed, copy-and-paste error
> (done in patch)
>
Agreed (just a comment).
>
> \commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\impl\SimpleLog.java
>
> Line 398: The javadoc below
> @see org.apache.commons.logging.Log#trace(Object, Throwable)
> should be
> @see org.apache.commons.logging.Log#trace(Object)
> (done in patch)
>
> \commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\LogFactory.java
> Line 142: Should be changed
> </code></pre> ---> </pre></code>
> (done in patch)
> Lines 335-340, 1130, 1219-1271: Indentation is wrong (is using
> tab-characters)
> (done in patch)
> Line 574: Should this be converted into a @todo?
Yes, possibly:
// TODO: think whether we need to handle exceptions from newFactory..
> Line 1172: LogFactory.class.getClassLoader.load(name) --->
> LogFactory.class.getClassLoader().load(name)
> (done in patch)
> Line 1217: The last part of the JavaDocs are cut in the middle of a
> sentence. What should it say?
* @return true if the <code>logFactoryClass</code> does extend
* <code>LogFactory</code> when that class is loaded
* via the same classloader that loaded the
* <code>logFactoryClass</code>.
> Lines 1462, 1466: The file doesn't have to be called FACTORY_PROPERTIES,
> the name is given by the parameter "fileName". The method is private and
> is only called once with fileName=FACTORY_PROPERTIES. Should this be
> changed?
Yes, 1462/1466 should use the fileName parameter when outputting their
diagnostic messages. Not serious, though because as you point out, the
fileName parameter always *is* FACTORY_PROPERTIES.
> Line 1477: OUTPUT_PROPERTY ---> DIAGNOSTICS_DEST_PROPERTY or perhaps
> "org.apache.commons.logging.diagnostics.dest". Which one is best?
Yep, javadoc wasn't updated. I think DIAGNOSTICS_DEST_PROPERTY is best;
it is a public constant so the user can see what real value is needed.
> Line 1655: Missing a space at the end of the string (after the colon)
> (done in patch)
>
>
> \commons-logging-1.1-RC10-src\xdocs\guide.xml
> Line 150: Remove " in each file".
> (done in patch)
>
>
> \commons-logging-1.1-RC10-src\xdocs\tech.xml
> Line 649:
> "JCL uses the context classloader to use the <code>Log</code>
> implementation."
> What should it be?
> "JCL uses the context classloader to get|load the <code>Log</code>
> implementation."
I think:
"to load the <code>Log</code> implementation".
We really do call ClassLoader.loadClass, so "load" seems right.
>
>
> \commons-logging-1.1-RC10-src\xdocs\troubleshooting.xml
> Line 91:
> "Each diagnostic message is prefixed with details of the class being
> logger in a standard format."
> What should it be?
> "Each diagnostic message is prefixed with details of the class being
> logged|loaded|diagnosed in a standard format."
>
Not sure. How about this?
"prefixed with details of the relevant class in a standard format."
Dennis, I'm really impressed by such careful inspection of the code!
Thanks for all the comments.
I'm +1 to the attached patch being applied. If you want to take care of
the other things mentioned here, great. Otherwise I will deal with them.
However I don't see anything here that's worth cancelling the RC10 vote
for. A few items would be good to put up on the wiki as "1.1 release
addendum".
Cheers,
Simon
========= (original patch follows)
> plain text document attachment (jcl-1.1-rc10.patch)
> Index: xdocs/guide.xml
> ===================================================================
> --- xdocs/guide.xml (revision 398696)
> +++ xdocs/guide.xml (arbetskopia)
> @@ -147,7 +147,7 @@
> When such a file exists, every entry in the properties file becomes an "attribute"
> of the LogFactory. When there is more than one such file in the classpath, releases
> of commons-logging prior to 1.1 simply use the first one found. From release 1.1,
> -each file may define a <code>priority</code> key in each file, and the file with
> +each file may define a <code>priority</code> key, and the file with
> the highest priority is used (no priority definition implies priority of zero).
> When multiple files have the same priority, the first one found is used.
> </p>
> Index: src/java/org/apache/commons/logging/impl/AvalonLogger.java
> ===================================================================
> --- src/java/org/apache/commons/logging/impl/AvalonLogger.java (revision 398661)
> +++ src/java/org/apache/commons/logging/impl/AvalonLogger.java (arbetskopia)
> @@ -248,7 +248,7 @@
> *
> * @param message to log.
> * @param t log this cause.
> - * @see org.apache.commons.logging.Log#debug(Object, Throwable)
> + * @see org.apache.commons.logging.Log#trace(Object, Throwable)
> */
> public void trace(Object message, Throwable t) {
> if (getLogger().isDebugEnabled()) getLogger().debug(String.valueOf(message), t);
> Index: src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
> ===================================================================
> --- src/java/org/apache/commons/logging/impl/LogFactoryImpl.java (revision 398661)
> +++ src/java/org/apache/commons/logging/impl/LogFactoryImpl.java (arbetskopia)
> @@ -758,8 +758,7 @@
> * or if no adapter at all can be instantiated
> */
> private Log discoverLogImplementation(String logCategory)
> - throws LogConfigurationException
> - {
> + throws LogConfigurationException {
> if (isDiagnosticsEnabled()) {
> logDiagnostic("Discovering a Log implementation...");
> }
> @@ -1248,8 +1247,7 @@
> current = current.getParent();
> }
>
> - // scan c2's ancestors to find c1
> - // scan c1's ancestors to find c2
> + // scan c2's ancestors to find c1
> current = c2;
> while (current != null) {
> if (current == c1)
> @@ -1319,7 +1317,7 @@
> * should not be recovered from.
> */
> private void handleFlawedHierarchy(ClassLoader badClassLoader, Class badClass)
> - throws LogConfigurationException {
> + throws LogConfigurationException {
>
> boolean implementsLog = false;
> String logInterfaceName = Log.class.getName();
> Index: src/java/org/apache/commons/logging/impl/SimpleLog.java
> ===================================================================
> --- src/java/org/apache/commons/logging/impl/SimpleLog.java (revision 398661)
> +++ src/java/org/apache/commons/logging/impl/SimpleLog.java (arbetskopia)
> @@ -395,7 +395,7 @@
> * <code>org.apache.commons.logging.impl.SimpleLog.LOG_LEVEL_TRACE</code>.
> *
> * @param message to log
> - * @see org.apache.commons.logging.Log#trace(Object, Throwable)
> + * @see org.apache.commons.logging.Log#trace(Object)
> */
> public final void trace(Object message) {
>
> Index: src/java/org/apache/commons/logging/LogFactory.java
> ===================================================================
> --- src/java/org/apache/commons/logging/LogFactory.java (revision 398661)
> +++ src/java/org/apache/commons/logging/LogFactory.java (arbetskopia)
> @@ -139,7 +139,7 @@
> * <strong>Note:</strong> <code>LogFactory</code> will print:
> * <code><pre>
> * [ERROR] LogFactory: Load of custom hashtable failed</em>
> - * </code></pre>
> + * </pre></code>
> * to system error and then continue using a standard Hashtable.
> * </p>
> * <p>
> @@ -328,16 +328,16 @@
> } catch (Throwable t) {
> // ignore
> if (!WEAK_HASHTABLE_CLASSNAME.equals(storeImplementationClass)) {
> - // if the user's trying to set up a custom implementation, give a clue
> - if (isDiagnosticsEnabled()) {
> - // use internal logging to issue the warning
> - logDiagnostic("[ERROR] LogFactory: Load of custom hashtable failed");
> - } else {
> - // we *really* want this output, even if diagnostics weren't
> + // if the user's trying to set up a custom implementation, give a clue
> + if (isDiagnosticsEnabled()) {
> + // use internal logging to issue the warning
> + logDiagnostic("[ERROR] LogFactory: Load of custom hashtable failed");
> + } else {
> + // we *really* want this output, even if diagnostics weren't
> // explicitly enabled by the user.
> - System.err.println("[ERROR] LogFactory: Load of custom hashtable failed");
> - }
> - }
> + System.err.println("[ERROR] LogFactory: Load of custom hashtable failed");
> + }
> + }
> }
> if (result == null) {
> result = new Hashtable();
> @@ -1127,7 +1127,7 @@
> // to their native logging system.
> //
> String msg =
> - "The application has specified that a custom LogFactory implementation should be used but " +
> + "The application has specified that a custom LogFactory implementation should be used but " +
> "Class '" + factoryClass + "' cannot be converted to '"
> + LogFactory.class.getName() + "'. ";
> if (implementsLogFactory) {
> @@ -1169,7 +1169,7 @@
> * classLoader was unable to load factoryClass.
> *
> * In either case, we call Class.forName, which is equivalent
> - * to LogFactory.class.getClassLoader.load(name), ie we ignore
> + * to LogFactory.class.getClassLoader().load(name), ie we ignore
> * the classloader parameter the caller passed, and fall back
> * to trying the classloader associated with this class. See the
> * javadoc for the newFactory method for more info on the
> @@ -1216,59 +1216,59 @@
> * @param logFactoryClass <code>Class</code> which may implement <code>LogFactory</code>
> * @return true if the <code>Class</code> is assignable from the
> */
> - private static boolean implementsLogFactory(Class logFactoryClass) {
> - boolean implementsLogFactory = false;
> - if (logFactoryClass != null) {
> - try {
> - ClassLoader logFactoryClassLoader = logFactoryClass.getClassLoader();
> - if (logFactoryClassLoader == null) {
> - logDiagnostic("[CUSTOM LOG FACTORY] was loaded by the boot classloader");
> - } else {
> - logHierarchy("[CUSTOM LOG FACTORY] ", logFactoryClassLoader);
> - Class factoryFromCustomLoader
> - = Class.forName("org.apache.commons.logging.LogFactory", false, logFactoryClassLoader);
> - implementsLogFactory = factoryFromCustomLoader.isAssignableFrom(logFactoryClass);
> - if (implementsLogFactory) {
> - logDiagnostic("[CUSTOM LOG FACTORY] " + logFactoryClass.getName()
> - + " implements LogFactory but was loaded by an incompatible classloader.");
> - } else {
> - logDiagnostic("[CUSTOM LOG FACTORY] " + logFactoryClass.getName()
> - + " does not implement LogFactory.");
> - }
> - }
> - } catch (SecurityException e) {
> - //
> - // The application is running within a hostile security environment.
> - // This will make it very hard to diagnose issues with JCL.
> - // Consider running less securely whilst debugging this issue.
> - //
> - logDiagnostic("[CUSTOM LOG FACTORY] SecurityException thrown whilst trying to determine whether " +
> - "the compatibility was caused by a classloader conflict: "
> - + e.getMessage());
> - } catch (LinkageError e) {
> - //
> - // This should be an unusual circumstance.
> - // LinkageError's usually indicate that a dependent class has incompatibly changed.
> - // Another possibility may be an exception thrown by an initializer.
> - // Time for a clean rebuild?
> - //
> - logDiagnostic("[CUSTOM LOG FACTORY] LinkageError thrown whilst trying to determine whether " +
> - "the compatibility was caused by a classloader conflict: "
> - + e.getMessage());
> - } catch (ClassNotFoundException e) {
> - //
> - // LogFactory cannot be loaded by the classloader which loaded the custom factory implementation.
> - // The custom implementation is not viable until this is corrected.
> - // Ensure that the JCL jar and the custom class are available from the same classloader.
> - // Running with diagnostics on should give information about the classloaders used
> - // to load the custom factory.
> - //
> - logDiagnostic("[CUSTOM LOG FACTORY] LogFactory class cannot be loaded by classloader which loaded the " +
> - "custom LogFactory implementation. Is the custom factory in the right classloader?");
> - }
> - }
> - return implementsLogFactory;
> - }
> + private static boolean implementsLogFactory(Class logFactoryClass) {
> + boolean implementsLogFactory = false;
> + if (logFactoryClass != null) {
> + try {
> + ClassLoader logFactoryClassLoader = logFactoryClass.getClassLoader();
> + if (logFactoryClassLoader == null) {
> + logDiagnostic("[CUSTOM LOG FACTORY] was loaded by the boot classloader");
> + } else {
> + logHierarchy("[CUSTOM LOG FACTORY] ", logFactoryClassLoader);
> + Class factoryFromCustomLoader
> + = Class.forName("org.apache.commons.logging.LogFactory", false, logFactoryClassLoader);
> + implementsLogFactory = factoryFromCustomLoader.isAssignableFrom(logFactoryClass);
> + if (implementsLogFactory) {
> + logDiagnostic("[CUSTOM LOG FACTORY] " + logFactoryClass.getName()
> + + " implements LogFactory but was loaded by an incompatible classloader.");
> + } else {
> + logDiagnostic("[CUSTOM LOG FACTORY] " + logFactoryClass.getName()
> + + " does not implement LogFactory.");
> + }
> + }
> + } catch (SecurityException e) {
> + //
> + // The application is running within a hostile security environment.
> + // This will make it very hard to diagnose issues with JCL.
> + // Consider running less securely whilst debugging this issue.
> + //
> + logDiagnostic("[CUSTOM LOG FACTORY] SecurityException thrown whilst trying to determine whether " +
> + "the compatibility was caused by a classloader conflict: "
> + + e.getMessage());
> + } catch (LinkageError e) {
> + //
> + // This should be an unusual circumstance.
> + // LinkageError's usually indicate that a dependent class has incompatibly changed.
> + // Another possibility may be an exception thrown by an initializer.
> + // Time for a clean rebuild?
> + //
> + logDiagnostic("[CUSTOM LOG FACTORY] LinkageError thrown whilst trying to determine whether " +
> + "the compatibility was caused by a classloader conflict: "
> + + e.getMessage());
> + } catch (ClassNotFoundException e) {
> + //
> + // LogFactory cannot be loaded by the classloader which loaded the custom factory implementation.
> + // The custom implementation is not viable until this is corrected.
> + // Ensure that the JCL jar and the custom class are available from the same classloader.
> + // Running with diagnostics on should give information about the classloaders used
> + // to load the custom factory.
> + //
> + logDiagnostic("[CUSTOM LOG FACTORY] LogFactory class cannot be loaded by classloader which loaded the " +
> + "custom LogFactory implementation. Is the custom factory in the right classloader?");
> + }
> + }
> + return implementsLogFactory;
> + }
>
> /**
> * Applets may run in an environment where accessing resources of a loader is
> @@ -1652,7 +1652,7 @@
> return;
> }
> if (classLoader != null) {
> - StringBuffer buf = new StringBuffer(prefix + "ClassLoader tree:");
> + StringBuffer buf = new StringBuffer(prefix + "ClassLoader tree: ");
> for(;;) {
> buf.append(objectId(classLoader));
> if (classLoader == systemClassLoader) {
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [VOTE] Release RC10 As Commons Logging 1.1
Posted by Dennis Lundberg <de...@apache.org>.
robert burrell donkin wrote:
> after some investigation, the issue which turned up with RC8 was
> determined not to be a bug. notes have been added to the troubleshooting
> documentation.
>
> RC10 is available from:
> http://people.apache.org/~rdonkin/commons-logging. please download,
> check and then vote.
<snip>
Sorry to be jumping into this at the last minute. I've done a code
review of /src/java and /xdocs and compared them to the 1.0.4 release.
As you might have seen I committed some spelling corrections to the
xdocs. The added documents are excellent by the way!
There were a couple of questions that popped up along the way, which I
will deal with below, one file at a time. Nothing big, mostly
documentation stuff, so nothing to hold up a release.
The attached patch file fixes a couple of them, while others need input
from you. I didn't want to commit the patch before I had checked it with
the rest of you.
I hope to be doing some practical tests in applet environments tomorrow.
\commons-logging-1.1-RC10-src\RELEASE-NOTES.txt
Lines 147-150:
"Previous releases of commons-logging-api.jar contained the Jdk14Logger
class;
this is now deprecated. It will be removed from the API jar in some future
release. If your application needs this jar, then instead of
upgrading to commons-logging-api-1.1.jar, upgrade to
commons-logging-1.1.jar."
I don't see why we advice this. The jar still contains Jdk14Logger.
Should we encourage people to switch jars now or when JCL 2 comes out?
Lines 173-174:
"File commons-logging-api-nn.jar provides no adapters to external logging
libraries, just the internally implemented SimpleLog and NoOpLog classes."
This is not entirely true. The Jdk14Logger is there as well.
There is nothing in the release notes about the fact that AvalonLogger
no longer implements Serializable. Do we need that?
\commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\impl\AvalonLogger.java
Line 251: Javadoc for trace(Object message, Throwable t) was changed
like this:
@see org.apache.commons.logging.Log#trace(java.lang.Object,
java.lang.Throwable) ---> @see
org.apache.commons.logging.Log#debug(Object, Throwable)
Should be changed back.
(done in patch)
\commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\impl\Jdk13LumberjackLogger.java
Line 42: A since-tag of "1.1" was added, but the class existed in SCM
when the previous version was released, but was not included in the jar.
Should the since tag be there?
\commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\impl\LogFactoryImpl.java
Line 121: The sentence seems to not have been finished...
Lines 173-175: Could use the constants that were defined in lines 74-78.
Line 761,1322: Missing indentation
(done in patch)
Line 1252: Should be removed, copy-and-paste error
(done in patch)
\commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\impl\SimpleLog.java
Line 398: The javadoc below
@see org.apache.commons.logging.Log#trace(Object, Throwable)
should be
@see org.apache.commons.logging.Log#trace(Object)
(done in patch)
\commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\LogFactory.java
Line 142: Should be changed
</code></pre> ---> </pre></code>
(done in patch)
Lines 335-340, 1130, 1219-1271: Indentation is wrong (is using
tab-characters)
(done in patch)
Line 574: Should this be converted into a @todo?
Line 1172: LogFactory.class.getClassLoader.load(name) --->
LogFactory.class.getClassLoader().load(name)
(done in patch)
Line 1217: The last part of the JavaDocs are cut in the middle of a
sentence. What should it say?
Lines 1462, 1466: The file doesn't have to be called FACTORY_PROPERTIES,
the name is given by the parameter "fileName". The method is private and
is only called once with fileName=FACTORY_PROPERTIES. Should this be
changed?
Line 1477: OUTPUT_PROPERTY ---> DIAGNOSTICS_DEST_PROPERTY or perhaps
"org.apache.commons.logging.diagnostics.dest". Which one is best?
Line 1655: Missing a space at the end of the string (after the colon)
(done in patch)
\commons-logging-1.1-RC10-src\xdocs\guide.xml
Line 150: Remove " in each file".
(done in patch)
\commons-logging-1.1-RC10-src\xdocs\tech.xml
Line 649:
"JCL uses the context classloader to use the <code>Log</code>
implementation."
What should it be?
"JCL uses the context classloader to get|load the <code>Log</code>
implementation."
\commons-logging-1.1-RC10-src\xdocs\troubleshooting.xml
Line 91:
"Each diagnostic message is prefixed with details of the class being
logger in a standard format."
What should it be?
"Each diagnostic message is prefixed with details of the class being
logged|loaded|diagnosed in a standard format."
--
Dennis Lundberg