You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Mark Thomas <ma...@apache.org> on 2013/10/30 18:52:10 UTC
[VOTE] Release Apache Commons Pool 2.0 RC1 as 2.0
Development on Pool 2 is complete and a review of the remaining DBCP
bugs has not highlighted any that are likely to trigger significant API
changes in Pool so it is time for a release.
The Pool 2.0 RC1 is available for review here:
https://dist.apache.org/repos/dist/dev/commons/pool/ (r3380)
Maven artifacts are here:
https://repository.apache.org/content/repositories/orgapachecommons-046/
Details of changes since 1.x are in the release notes:
https://dist.apache.org/repos/dist/dev/commons/pool/RELEASE-NOTES.txt
http://people.apache.org/~markt/dev/commons-pool-2.0-RC1/changes-report.html
The tag is here:
http://svn.apache.org/repos/asf/commons/proper/foo/tags/POOL_2_0_RC1/
(r 1537146)
Site:
http://people.apache.org/~markt/dev/commons-pool-2.0-RC1
(There are various broken links - I'm in the process of working out if
any of them are going to require a new release)
KEYS:
http://www.apache.org/dist/commons/KEYS
Please review the release candidate and vote.
This vote will close no sooner that 72 hours from now
[ ] +1 Release these artifacts
[ ] +0 OK, but...
[ ] -0 OK, but really should fix...
[ ] -1 I oppose this release because...
Cheers,
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[CANCELLED][VOTE] Release Apache Commons Pool 2.0 RC1 as 2.0
Posted by Mark Thomas <ma...@apache.org>.
A few issues have been found and fixed with RC1 so I am cancelling this
vote. I'll start the process for RC2 shortly.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Apache Commons Pool 2.0 RC1 as 2.0
Posted by sebb <se...@gmail.com>.
On 31 October 2013 12:05, Mark Thomas <ma...@apache.org> wrote:
> On 30/10/2013 21:36, sebb wrote:
>> On 30 October 2013 19:31, Mark Thomas <ma...@apache.org> wrote:
>>> On 30/10/2013 17:52, Mark Thomas wrote:
>>>
>>> <snip/>
>>>
>>>> Please review the release candidate and vote.
>>>> This vote will close no sooner that 72 hours from now
>>>>
>>>> [ ] +1 Release these artifacts
>>>> [ ] +0 OK, but...
>>>> [ ] -0 OK, but really should fix...
>>>> [X] -1 I oppose this release because...
>>>
>>> I've just changed the API to break a cyclic dependency. There is
>>> definitely going to need to be another RC. I'll leave this vote open for
>>> now to give folks a chance to find any other issues.
>>
>> The release notes mention major changes to the API, but don't mention
>> that the package name and Maven coordinates have changed from the
>> previous pool release.
>
> Added.
Thanks.
Might perhaps be worth mentioning the previous package name and Maven coords?
> The RN should be clear that the new features,
>> bug fixes etc relate to version 1.? of the code (whatever that is).
>
> Added.
>
>> Also, there are a lot of instances of variable hiding, mainly caused
>> by local copies with the same name.
>> The ones I have checked seem to be harmless, but it may not always be
>> clear whether the code should be using the local copy or the hidden
>> copy.
>
> If there is a local copy, the code should be using it. The name clashes
> aren't something that particularly bothers me. I see you have fixed them
> - that works for me.
OK.
It's not unknown for name hiding to cause bugs, which is why I think
it's worth fixing.
A while back I found some code that accidentally used a local variable
when it should have updated the class field.
>> Also, the ones in LinkedBlockingDeque are completely unnecessary, for example:
>
> The code was copied from Harmony and deliberately only changed where
> necessary to expose the additional information required by pool. Not
> making other changes when copying code like this is a habit I've picked
> up to make it easy to sync future changes to the code. Given that future
> changes are unlikely to Harmony, I have no objection to you cleaning it
> up if you want to scratch that particular itch.
OK, done.
>> Given that this is the first release of a new package/Maven coords, it
>> would be sensible to ensure that any internal classes are clearly
>> marked as such, as that would allow them to be changed without
>> breaking the public API.
>
> This should already be the case with internal use only classes made
> package private. I found a few errors which I have fixed. Did you have
> anything specific in mind?
It was a general remark, I had no specific classes in mind.
BTW, as you will have seen, I updated the pom so the download page can
be created automatically in future.
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Apache Commons Pool 2.0 RC1 as 2.0
Posted by Mark Thomas <ma...@apache.org>.
On 30/10/2013 21:36, sebb wrote:
> On 30 October 2013 19:31, Mark Thomas <ma...@apache.org> wrote:
>> On 30/10/2013 17:52, Mark Thomas wrote:
>>
>> <snip/>
>>
>>> Please review the release candidate and vote.
>>> This vote will close no sooner that 72 hours from now
>>>
>>> [ ] +1 Release these artifacts
>>> [ ] +0 OK, but...
>>> [ ] -0 OK, but really should fix...
>>> [X] -1 I oppose this release because...
>>
>> I've just changed the API to break a cyclic dependency. There is
>> definitely going to need to be another RC. I'll leave this vote open for
>> now to give folks a chance to find any other issues.
>
> The release notes mention major changes to the API, but don't mention
> that the package name and Maven coordinates have changed from the
> previous pool release.
Added.
The RN should be clear that the new features,
> bug fixes etc relate to version 1.? of the code (whatever that is).
Added.
> Also, there are a lot of instances of variable hiding, mainly caused
> by local copies with the same name.
> The ones I have checked seem to be harmless, but it may not always be
> clear whether the code should be using the local copy or the hidden
> copy.
If there is a local copy, the code should be using it. The name clashes
aren't something that particularly bothers me. I see you have fixed them
- that works for me.
> Also, the ones in LinkedBlockingDeque are completely unnecessary, for example:
The code was copied from Harmony and deliberately only changed where
necessary to expose the additional information required by pool. Not
making other changes when copying code like this is a habit I've picked
up to make it easy to sync future changes to the code. Given that future
changes are unlikely to Harmony, I have no objection to you cleaning it
up if you want to scratch that particular itch.
> Given that this is the first release of a new package/Maven coords, it
> would be sensible to ensure that any internal classes are clearly
> marked as such, as that would allow them to be changed without
> breaking the public API.
This should already be the case with internal use only classes made
package private. I found a few errors which I have fixed. Did you have
anything specific in mind?
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Apache Commons Pool 2.0 RC1 as 2.0
Posted by sebb <se...@gmail.com>.
On 30 October 2013 19:31, Mark Thomas <ma...@apache.org> wrote:
> On 30/10/2013 17:52, Mark Thomas wrote:
>
> <snip/>
>
>> Please review the release candidate and vote.
>> This vote will close no sooner that 72 hours from now
>>
>> [ ] +1 Release these artifacts
>> [ ] +0 OK, but...
>> [ ] -0 OK, but really should fix...
>> [X] -1 I oppose this release because...
>
> I've just changed the API to break a cyclic dependency. There is
> definitely going to need to be another RC. I'll leave this vote open for
> now to give folks a chance to find any other issues.
The release notes mention major changes to the API, but don't mention
that the package name and Maven coordinates have changed from the
previous pool release. The RN should be clear that the new features,
bug fixes etc relate to version 1.? of the code (whatever that is).
Also, there are a lot of instances of variable hiding, mainly caused
by local copies with the same name.
The ones I have checked seem to be harmless, but it may not always be
clear whether the code should be using the local copy or the hidden
copy.
Also, the ones in LinkedBlockingDeque are completely unnecessary, for example:
public boolean offerFirst(E e) {
if (e == null) throw new NullPointerException();
final ReentrantLock lock = this.lock; // <== local variable
with same name
lock.lock();
try {
return linkFirst(e);
} finally {
lock.unlock();
}
}
this.lock is final (obviously) so there is absolutely no need to take
a copy of the pointer.
There are some other cases where a volatile variable is copied, but
the copy should have a new name so it is clear which copy is intended
to be used.
Given that this is the first release of a new package/Maven coords, it
would be sensible to ensure that any internal classes are clearly
marked as such, as that would allow them to be changed without
breaking the public API.
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Apache Commons Pool 2.0 RC1 as 2.0
Posted by Mark Thomas <ma...@apache.org>.
On 30/10/2013 19:52, Benedikt Ritter wrote:
> On my machine the tests take forever to run, about 5 minutes for mvn test.
> I guess this is normal? (haven't looked into the tests in detail)
Yes, that is normal. A number of the tests check various timeouts
(eviction, abandoned objects etc.) so they sleep while waiting for the
timeouts to expire.
> Test coverage in the proxy package is relatively low (58%) but since I'm
> not involved in the development I don't know how easy/hard it is to improve
> coverage.
Not too hard. I've added a few more unit tests.
> FindBugs Found reliance on default encoding in new
> org.apache.commons.pool2.impl.AbandonedConfig(): new
> java.io.PrintWriter(OutputStream). An exclusion with an explanatory comment
> could be added for RC 2.
I'd already spotted and fixed this one.
> We had this discussion [1] about Configuration 1.10 regard copy rights and
> naming (there was another thread about this, but I can't find it
> currently). Looks like the commons Pool logo is missing a trade mark.
I'm no graphics designer so I just copied the TM from the BeanUtils library.
> If I
> understand correctly every mentioning of "pool", and "Commons pool" has to
> be changed to "the Apache Commons Pool component".
The brand guidelines aren't quite that rigid but I suspect the docs are
in need of a review. I've taken a first look. Some more tweaks will
probably be required.
I prefer s/component/library/
> The RELEASE_NOTES.txt say:
>
> The Apache Commons Pool team is pleased to announce the release of
> commons-pool2-2.0
>
> This is probably generated from maven project.name. Sounds strange to me
> because the head line says "Apache Commons Pool 2.0 RELEASE NOTES"
Fixed.
> One minor thing: the vote mail contained a wrong URL for the tag. It said
> http://svn.apache.org/repos/asf/commons/proper/foo/tags/POOL_2_0_RC1/
> where is should have been
> http://svn.apache.org/repos/asf/commons/proper/pool/tags/POOL_2_0_RC1/<http://svn.apache.org/repos/asf/commons/proper/foo/tags/POOL_2_0_RC1/>
That will be because I copied and pasted the text from the step-by-step
guide and forgot to fix that URL. I'll fix it for the next RC.
Thanks for the feedback.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Apache Commons Pool 2.0 RC1 as 2.0
Posted by Benedikt Ritter <br...@apache.org>.
Hello Mark,
Builds fine with:
Apache Maven 3.1.1 (0728685237757ffbf44136acec0402957f723d9a; 2013-09-17
17:22:22+0200)
Maven home: /Applications/dev/maven/apache-maven-3.1.1
Java version: 1.7.0_40, vendor: Oracle Corporation
Java home:
/Library/Java/JavaVirtualMachines/jdk1.7.0_40.jdk/Contents/Home/jre
Default locale: de_DE, platform encoding: UTF-8
OS name: "mac os x", version: "10.9", arch: "x86_64", family: "mac"
Side looks good,
signs are good,
archives and tag have the same content
On my machine the tests take forever to run, about 5 minutes for mvn test.
I guess this is normal? (haven't looked into the tests in detail)
Test coverage in the proxy package is relatively low (58%) but since I'm
not involved in the development I don't know how easy/hard it is to improve
coverage.
FindBugs Found reliance on default encoding in new
org.apache.commons.pool2.impl.AbandonedConfig(): new
java.io.PrintWriter(OutputStream). An exclusion with an explanatory comment
could be added for RC 2.
We had this discussion [1] about Configuration 1.10 regard copy rights and
naming (there was another thread about this, but I can't find it
currently). Looks like the commons Pool logo is missing a trade mark. If I
understand correctly every mentioning of "pool", and "Commons pool" has to
be changed to "the Apache Commons Pool component".
The RELEASE_NOTES.txt say:
The Apache Commons Pool team is pleased to announce the release of
commons-pool2-2.0
This is probably generated from maven project.name. Sounds strange to me
because the head line says "Apache Commons Pool 2.0 RELEASE NOTES"
One minor thing: the vote mail contained a wrong URL for the tag. It said
http://svn.apache.org/repos/asf/commons/proper/foo/tags/POOL_2_0_RC1/
where is should have been
http://svn.apache.org/repos/asf/commons/proper/pool/tags/POOL_2_0_RC1/<http://svn.apache.org/repos/asf/commons/proper/foo/tags/POOL_2_0_RC1/>
Thanks,
Benedikt
[1] http://markmail.org/thread/bmyypnqnkfcg3wgp
2013/10/30 Mark Thomas <ma...@apache.org>
> On 30/10/2013 17:52, Mark Thomas wrote:
>
> <snip/>
>
> > Please review the release candidate and vote.
> > This vote will close no sooner that 72 hours from now
> >
> > [ ] +1 Release these artifacts
> > [ ] +0 OK, but...
> > [ ] -0 OK, but really should fix...
> > [X] -1 I oppose this release because...
>
> I've just changed the API to break a cyclic dependency. There is
> definitely going to need to be another RC. I'll leave this vote open for
> now to give folks a chance to find any other issues.
>
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
Re: [VOTE] Release Apache Commons Pool 2.0 RC1 as 2.0
Posted by Mark Thomas <ma...@apache.org>.
On 30/10/2013 17:52, Mark Thomas wrote:
<snip/>
> Please review the release candidate and vote.
> This vote will close no sooner that 72 hours from now
>
> [ ] +1 Release these artifacts
> [ ] +0 OK, but...
> [ ] -0 OK, but really should fix...
> [X] -1 I oppose this release because...
I've just changed the API to break a cyclic dependency. There is
definitely going to need to be another RC. I'll leave this vote open for
now to give folks a chance to find any other issues.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org