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

[Naoto Sato]

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
>>>
>>>
>>>
>