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