Review Request: Warnings cleanup in sun.management and its subpackages (original) (raw)

Mandy Chung mandy.chung at oracle.com
Mon Dec 5 17:42:24 PST 2011

On 12/5/11 4:49 PM, Stuart Marks wrote:

Hi Mandy,

Thanks for the thorough review. For warnings cleanup, we've been trying to avoid including refactorings like changing Enumerations to Iterator or to the enhanced-for loop. Changes to use try-with-resources also fall into this category. But I've been OK with such changes if the primary maintainer of the area is OK with them.

I agree that we should avoid refactoring from warnings cleanup in general. So I try to be cautious about where to suggest replacing the iterator with foreach (this is the main suggestion) and also keep them to the lines that Kurchi already modified.

I no longer work on this area but I'm sure the serviceability team will appreciate this cleanup.

I wasn't sure to suggest try-with-resources initially but Kurchi already included such change in her webrev. So the testing required for her change should cover it :-)

Are you comfortable giving the go-ahead for these kinds of changes? I assume you are, since you suggested them, but I just wanted to double check.

I'm comfortable for the places suggested in my comment as they are trivial ones. I would not recommend to look for other refactoring opportunity besides them (e.g. the snmp files are more convoluted).

Also, would any additional testing be required before these changes go in?

Thanks for asking. I meant to mention it - jdk_lang and jdk_management test targets. java.lang.management tests are included in the jdk_lang target. jdk_management includes the sun.management tests and it also includes jmx tests which Kurchi's change didn't touch that area but no harm to run that and make the jprt submission simpler.

Thanks Mandy

More information about the jdk8-dev mailing list