<i18n dev> [15] RFR: 8187987: Add a mechanism to configure custom variants in HijrahChronology

[Joe Wang]

On 1/14/20 6:04 PM, naoto.sato at oracle.com wrote:
> Hi Joe,
>
> Thank you for the review. Please find my comments below:
>
> On 1/14/20 3:35 PM, Joe Wang wrote:
>> Hi Naoto,
>>
>> Since it's dealing with non-standard properties files, is there a 
>> need to verify the files? The constructor (HijrahChronology) does 
>> check whether the id or type is empty. If there is no existing 
>> process to validate, it's probably not worth it to spend time as it's 
>> rare and it's config time.
>
> IIUC, the idea is to create the instance at class loading time (thus 
> the faster the better) and cache it, then check the validity later at 
> actual method invocation (cf. checkCalendarInit() method).

Make sense.
>
>>
>> The test summary states "Test image creation", it may be better to 
>> say sth. like verifies custom configuration?
>
> Good catch! I was simply copying some portion from other test case. 
> Corrected:
>
> https://cr.openjdk.java.net/~naoto/8187987/webrev.01/

Looks good to me.

Best regards,
Joe

>
> Naoto
>
>>
>> Best,
>> Joe
>>
>> On 1/14/20 8:35 AM, naoto.sato at oracle.com wrote:
>>> Hi,
>>>
>>> Please review the fix to the following issue:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8187987
>>>
>>> The proposed CSR and changeset are located at:
>>>
>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8236810
>>> Webrev: https://cr.openjdk.java.net/~naoto/8187987/webrev.00/
>>>
>>> The spec of java.time.chrono.HijrahChronology suggests that it could 
>>> load custom variants of Hijirah calendar type using properties 
>>> files. However it does not work as designed. This change intends to 
>>> make it work.
>>>
>>> Naoto
>>