Thanks, Lance.
On 11/14/17 2:34 PM, Lance Andersen wrote:
Hi Naoto
DateTimeFormatter.java, DateTimeFormatterBuilder.java, WeekFields.java.,
Calendar.java, Currency.java (and others)
——————
136 * the chronology and/or the zone are also overriden. If both "ca"
and "rg" are
137 * specified, the chronology from "ca" extension supersedes the
implicit one
138 * from "rg" extension.
————————
I would consider changing 137 “… from the “ca extension” and “… from the
“rg” extension"
I agree wth Rogers suggestion to use @{code} for the new javadoc if at
all possible
Will change them.
The tests look reasonable. I was curious why a few of the new tests
such as CalendarDataTest.java are not using TestNG where others are.
For the new tests in the new "bcp47" directory, I used TestNG, for other
tests in existing directories, I followed the test method in those
directories. Not from any definitive reason.
Naoto
On Nov 14, 2017, at 4:28 PM, Roger Riggs <roger.ri...@oracle.com
<mailto:roger.ri...@oracle.com>> wrote:
Hi Naoto,
A few comments, there is lots here to review.
tools/cldrconverter/LDMLParseHandler.java:
- 948: TODO?
DateFormatSymbols.java:
- 88: "overriden" -> "overridden"
DecimalFormatSymbols:
- 60: "rg" -> "nu"?
- 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"
java/util/Calendar.java:
138 * They may also be specified explicitly through the methods for
setting their
139 * values.
java/util/spi/LocaleNameProvider.java:
167, 193: looks a bit odd to return null but maybe that's the default
if not overridden
Yeah, could you explain when null would not be returned, or is this just
stubbed out code for now?
It looks like a few misc changes are included too:
- LauncherHelper: 271: getDisplayName()...
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
(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
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
