Daniel,

Thanks for the review.

On 10/9/2013 12:38 AM, Daniel Fuchs wrote:

This looks good - but I think you could move the changes line 554-562 and
put them back inside the KnownLevel constructor where they were before.
This would allow you to keep mirroredLevel final.


That's right. I separated those lines in any earlier version and later added the private constructor per your suggestion.

Small nit: the name 'custom' is a misnomer - as it will be true for both standard and
custom levels...

I renamed it to "visible" (visible levels can be found by Level.parse)


Concerning the test I don't think you need to copy the property file to test.classes,
because I believe jprt puts test.src inside the classpath.

I think you meant jtreg.  Thanks I have fixed that.

(another possibility would be to use a custom subclass of ListResourceBundle instead)

Finally, I think that test needs to be run in main/othervm mode - otherwise it might
fail intermittently...

Another good catch.  Fixed.

The updated webrev:
http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8026027/webrev.01/

Mandy

Reply via email to