please review 7117612: warnings fixes in java.lang (original) (raw)

Stuart Marks stuart.marks at oracle.com
Thu Dec 8 06:22:09 UTC 2011

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.

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).

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.

Thanks for working on this!

s'marks

More information about the core-libs-dev mailing list