please review 7117612: warnings fixes in java.lang
omajid at redhat.com
Thu Dec 8 16:25:21 UTC 2011
On 12/08/2011 01:22 AM, Stuart Marks wrote:
> On 12/7/11 3:13 PM, Omair Majid wrote:
>> On 12/07/2011 05:43 AM, Alan Bateman wrote:
>>> I looked through the latest webrev. In
>>> EnumConstantNotPresentException.java then the
>>> @SuppressWarnings("rawtypes") should probably have a comment to explain
>>> why it is needed. In ThreadLocal.get then it's a pity that an additional
>>> local is needed to increase the size of the method. Otherwise the
>>> changes look okay to me.
>> Updated webrev at:
>> Is there something I should do to address the extra local, or is it
>> good to go?
> Hi Omair,
> Everything looks good to me. I think Alan was lamenting that adding the
> local variable for the sole purpose of adding the @SuppressWarnings
> annotation makes the method appear longer and more complex. The
> alternative is to put @SuppressWarnings on the entire method, which
> we've consistently frowned on, so I don't see the need to change anything.
Thanks for the clarification. Since this is the JDK code, I wasnt sure
if this is something that might lead to a problem.
> By the way, I've also run this changeset through our internal
> multi-platform build and test system, and everything works fine (with
> the exception of intermittent failures unrelated to this change).
That's great to know. I feel a little more confident about this
> You have commit rights, don't you? I'd say it's OK to proceed with the
> push. Or, if you prefer, Alan should be online in just a couple hours,
> and I'm sure he can give the final go-ahead.
Yes, I do have commit rights and I would be more than happy to push the
changeset. One quick question, if you dont mind. Does this comment look
7117612: Miscellaneous warnings in java.lang
Reviewed-by: smarks, dholmes, alanb
Shall I add Joe Darcy to the list of reviewers too? He did make a
comment on AutoClosable, but I am not sure if he was a 'reviewer'.
> Thanks for working on this!
Glad I could help out.
More information about the core-libs-dev