On 09/10/2013 11:41, Daniel Fuchs wrote:
Hi Mandy,
This looks good!
+1
-Chris.
(not a reviewer)
-- daniel
On 10/9/13 12:29 PM, Mandy Chung wrote:
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