Hi Alan,

Thanks for the review...

On 10/20/2014 11:07 AM, Alan Bateman wrote:
On 20/10/2014 15:53, roger riggs wrote:
Please review for JDK 9 only.

To aid the modularization effort, the configuration of the Hijrah calendar
should move the Hijrah calendar data to a resource.

At this point, it does not look like there will be other Hijrah calendar
variants; the function of calendar.properties to configure variants
is unnecessary and is proposed to be removed.

Since the other uses of calendars.properties have been eliminated
the calendars.properties is removed.

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-hijrah-config-8049376/
Issue:
  https://bugs.openjdk.java.net/browse/JDK-8048124
This mostly looks good to me. I just wonder about the removal of the doPrivileged in readConfigProperties where you will get null if there is something on the stack with restricted permissions.
What permission would be needed to read the resource?
The limited doPrivileged should include the minimum permission.

A minor comment on the @implNote in HijrahChronology is that it has more than I would expect, it might be simpler to just say that hijrah-config-<calendar-type>.properties is loaded as a resource file. Also the import of java.lang.String looks unnecessary.
ok, will correct.

Roger


-Alan.



Reply via email to