commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: [VOTE] Release Apache Commons Pool 2.0 RC1 as 2.0
Date Thu, 31 Oct 2013 12:05:24 GMT
On 30/10/2013 21:36, sebb wrote:
> On 30 October 2013 19:31, Mark Thomas <markt@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


Mime
View raw message