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