Return-Path: X-Original-To: apmail-commons-dev-archive@www.apache.org Delivered-To: apmail-commons-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 613A51093B for ; Thu, 31 Oct 2013 12:05:36 +0000 (UTC) Received: (qmail 40229 invoked by uid 500); 31 Oct 2013 12:05:33 -0000 Delivered-To: apmail-commons-dev-archive@commons.apache.org Received: (qmail 39968 invoked by uid 500); 31 Oct 2013 12:05:32 -0000 Mailing-List: contact dev-help@commons.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Commons Developers List" Delivered-To: mailing list dev@commons.apache.org Received: (qmail 39958 invoked by uid 99); 31 Oct 2013 12:05:32 -0000 Received: from minotaur.apache.org (HELO minotaur.apache.org) (140.211.11.9) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 31 Oct 2013 12:05:32 +0000 Received: from localhost (HELO [192.168.23.9]) (127.0.0.1) (smtp-auth username markt, mechanism plain) by minotaur.apache.org (qpsmtpd/0.29) with ESMTP; Thu, 31 Oct 2013 12:05:31 +0000 Message-ID: <52724784.9090304@apache.org> Date: Thu, 31 Oct 2013 12:05:24 +0000 From: Mark Thomas User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 MIME-Version: 1.0 To: Commons Developers List Subject: Re: [VOTE] Release Apache Commons Pool 2.0 RC1 as 2.0 References: <5271474A.1060200@apache.org> <52715E8B.7020809@apache.org> In-Reply-To: X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On 30/10/2013 21:36, sebb wrote: > On 30 October 2013 19:31, Mark Thomas wrote: >> On 30/10/2013 17:52, Mark Thomas wrote: >> >> >> >>> 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