Review request for 8026027: Level.parse should return the custom Level instance instead of the mirrored Level

[Daniel Fuchs]

Hi Mandy,

This looks good!

(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