On 1/14/20 6:04 PM, naoto.s...@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.s...@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
