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

naoto.sato at oracle.com naoto.sato at oracle.com
Wed Jan 15 02:04:57 UTC 2020


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).

> 
> 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/

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
> 


More information about the i18n-dev mailing list