[10] RFR 8176841: Additional Unicode Language-Tag Extensions

[Lance Andersen]

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

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.

> On Nov 14, 2017, at 4:28 PM, Roger Riggs <Roger.Riggs at 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.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>