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

[Chris Hegarty]

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
>