Review Request: Warnings cleanup in sun.management and its subpackages

[Mandy Chung]

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