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

[Naoto Sato]

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.Riggs at oracle.com 
>> <mailto: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>
> 
> 
>