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-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
