please review 7117612: warnings fixes in java.lang

Omair Majid 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:
>> http://cr.openjdk.java.net/~omajid/webrevs/warnings-day-2011/04/
>>
>> 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 
changeset now.

> 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 
fine?

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.

Cheers,
Omair



More information about the core-libs-dev mailing list