Thanks, Roger.
On 11/14/17 1:28 PM, Roger Riggs wrote:
Hi Naoto,
A few comments, there is lots here to review.
tools/cldrconverter/LDMLParseHandler.java:
- 948: TODO?
Will remove it.
DateFormatSymbols.java:
- 88: "overriden" -> "overridden"
DecimalFormatSymbols:
- 60: "rg" -> "nu"?
"nu" is mentioned in each constructor. "rg" is correct here.
- 62: "overriden" -> "overridden"
NumberFormat.java:
- 106: "a <code>NumberFormat</code>" -> "{@code NumberFormat}
java/time/format/DateTimeFormatter.java:
- 136, 561, 589, 1463: "overriden" -> "overridden"
- 1486: long line, wrap please
DateTimeFormatterBuilder.java:
- 2168,2194: - 62: "overriden" -> "overridden"
All of those typos will be corrected.
java/util/Calendar.java:
138 * They may also be specified explicitly through the methods for
setting their
139 * values.
What do you suggest here?
java/util/spi/LocaleNameProvider.java:
167, 193: looks a bit odd to return null but maybe that's the default
if not overridden
Yes, that's the default, and if it's null, then fall back to other
possibilities, either parent locale or other locale provider.
It looks like a few misc changes are included too:
- LauncherHelper: 271: getDisplayName()...
I had discussed this with Kumar, and he agreed to fix this.
CalendarDataProviderImpl.java:
- 51, 58: Should these limit the value to 0-6 to avoid odd
arithmetic; (Throw an exception if out of range)
LocaleNameProviderImpl.java:
- 176/186: Did you want to call super to do the RequiresNonNull
checks? Or will the NPEs happen naturally.
SPILocaleProviderAdapter.java:
- 578, 585: assert seems like of useless here; if null, will NPE; only
if asserts are enabled behavior will change to AssertError
Will address those in the next webrev.
Naoto
(I have not looked at the tests)
Regards, Roger
On 11/9/2017 6:34 PM, Naoto Sato wrote:
Kindly requesting reviews. I incorporated a fix to the following issue
raised by the test team:
https://bugs.openjdk.java.net/browse/JDK-8190918
Here is the updated webrev:
http://cr.openjdk.java.net/~naoto/8176841.8189134.8190918/webrev.04/
And the webrev since the one below (to address 8190918):
http://cr.openjdk.java.net/~naoto/8190918/
Naoto
On 11/2/17 2:42 PM, Naoto Sato wrote:
Hello,
Please review the proposed changes for the following issues:
8176841: Additional Unicode Language-Tag Extensions
8189134: New system properties for the default Locale extensions
The proposed changeset is located at:
http://cr.openjdk.java.net/~naoto/8176841/webrev.03/
This serves as the implementation of JEP 314.
Naoto
