Review Request: Warnings cleanup in sun.management and its subpackages
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