<i18n dev> [9] RFR: 8008577: Use CLDR Locale Data by Default (JEP-252)

[Masayoshi Okutsu]

Looks good to me.

Masayoshi

On 6/25/2015 1:15 AM, Naoto Sato wrote:
> Thanks. Here is the diff from "webrev.01" to address your comment:
>
> http://hg.openjdk.java.net/jdk9/sandbox/jdk/rev/b8faab65bb62
>
> Naoto
>
> On 6/24/15 2:16 AM, Masayoshi Okutsu wrote:
>> applyParentLocales() sets parentLocalesMap before populating the map
>> with data. It's possible that other threads look up the map without the
>> (full) data. So, a Map (local variable) should be populated and then
>> parentLocalesMap should be set to the Map. Also, parentLocalesMap needs
>> to be volatile or synchronized should be used when setting it. I don't
>> think it's necessary to use ConcurrentHashMap if the map is not modified
>> after its initialization.
>>
>> Otherwise, the changes look good to me.
>>
>> Masayoshi
>>
>> On 6/24/2015 6:56 AM, Naoto Sato wrote:
>>> Here is the updated webrev:
>>>
>>> http://cr.openjdk.java.net/~naoto/8008577/webrev.01/
>>>
>>> As to the 3rd comment below, I did not modify it because that would
>>> simply duplicate the same piece of code in each getCandidateLocales()
>>> implementation (from the current location).
>>>
>>> Naoto
>>>
>>> On 6/19/15 1:53 AM, Masayoshi Okutsu wrote:
>>>> Sorry for taking time. Here are my comments.
>>>>
>>>> src/jdk.localedata/share/classes/sun/text/resources/*JavaTimeSupplementary*.java: 
>>>>
>>>>
>>>>
>>>>   - The year range of the first line of the copyright header should be
>>>> "2015," for the new ones and "2013, 2015," for the updated ones.
>>>>   - I wonder if JavaTimeSupplementary*.java contain some CR-LF lines
>>>> because some line spacing is wide in the webrev. (hard to determine 
>>>> that
>>>> from the webrev.)
>>>>
>>>> src/java.base/share/classes/sun/util/cldr/CLDRLocaleProviderAdapter.java: 
>>>>
>>>>
>>>>   - applyParentLocales() needs to be thread-safe with 
>>>> parentLocalesMap?
>>>>
>>>> src/java.base/share/classes/sun/util/resources/LocaleData.java:
>>>>   - I wonder if the ResourceBundleBasedAdapter.getCandidateLocales()
>>>> implementations should return an optimized candidate list rather than
>>>> removing unsupported Locales from the list there.
>>>>
>>>> test/sun/text/resources/LocaleDataTest.java:
>>>>
>>>> + * -cldr option specifies to test CLDR locale data. The default data
>>>> file name for this
>>>> + * option is "CLDRLocaleData".
>>>>
>>>>   - The file name should be "LocaleData.cldr"?
>>>>
>>>> test/java/text/Format/DecimalFormat/RoundingAndPropertyTest.java:
>>>>   - added the bug id, but no changes to the test?
>>>>
>>>> Thanks,
>>>> Masayoshi
>>>>
>>>> On 6/9/2015 5:58 AM, Naoto Sato wrote:
>>>>> Hello,
>>>>>
>>>>> Please review the proposed changes for 8008577[1], the implementation
>>>>> of the JEP-252[2]. The proposed changes are located at:
>>>>>
>>>>> http://cr.openjdk.java.net/~naoto/8008577/webrev.00/
>>>>>
>>>>> Here are the very high level summary of changes:
>>>>>
>>>>> - Now the default locale provider order is CLDR,JRE,SPI.
>>>>> - The CLDR data is upgraded from 21.0.1 to 27.0.0
>>>>> - According the CLDR upgrade, the converter tool and the runtime are
>>>>> now capable of "alias" and "parentLocales" tags.
>>>>> - Regression tests that are specific to the existing JRE locale data
>>>>> are now run specifically with "JRE,SPI" providers.
>>>>>
>>>>> I would like the build group to review the build changes mainly with
>>>>> the CLDR changes, and i18n group for the rest.
>>>>>
>>>>> Naoto
>>>>> -- 
>>>>> [1]: https://bugs.openjdk.java.net/browse/JDK-8008577
>>>>> [2]: https://bugs.openjdk.java.net/browse/JDK-8043554
>>>>
>>