<i18n dev> Review Request:JDK-8163350-LocaleProviderAdapter Preference list retrieved is wrong, when -Djava.locale.providers=COMPAT

Masayoshi Okutsu masayoshi.okutsu at oracle.com
Mon Aug 22 05:47:29 UTC 2016


Looks good to me.

Masayoshi


On 8/22/2016 1:10 PM, Rachna Goel wrote:
> Hi Masayoshi,
>
> Thanks for the review.
>
> please have a look at updated webrev
>
> http://cr.openjdk.java.net/~rgoel/jdk-8163350/webrev.04/
>
> Thanks,
>
> Rachna
>
>
> On 8/16/16 1:16 PM, Masayoshi Okutsu wrote:
>> Hi Rachna,
>>
>> The fix looks good to me. But the test should be changed.
>>
>> - It's unnecessary to statically import 
>> sun.util.locale.provider.LocaleProviderAdapter.Type.
>> - Variable name PreferenceList should be preferenceList.
>> - No need to initialize preferenceList with an ArrayList.
>>
>> Thanks,
>> Masayoshi
>>
>> On 8/10/2016 2:31 AM, Rachna Goel wrote:
>>> Hi Naoto,
>>>
>>> Thanks for the review.
>>>
>>> Please have a look at updated patch:
>>>
>>> http://cr.openjdk.java.net/~rgoel/jdk-8163350/webrev.03/
>>>
>>> Thanks,
>>>
>>> Rachna
>>>
>>>
>>>
>>> On 8/9/16 9:39 PM, Naoto Sato wrote:
>>>> Hi Rachna,
>>>>
>>>> I should've caught it at the internal review, but here are some 
>>>> cosmetic comments to the test case.
>>>>
>>>> - I'd prefer a blank line between import statements and the main 
>>>> method.
>>>> - The argument to main() should be (String[] args)
>>>> - Indentation is incorrect at line 39
>>>> - Needs parens for the if statement at line 41.
>>>>
>>>> Naoto
>>>>
>>>> On 8/9/16 3:51 AM, Rachna Goel wrote:
>>>>> Hi,
>>>>>
>>>>> Please review fix for JDK-8163350.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8163350
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~rgoel/jdk-8163350/webrev.02/
>>>>>
>>>>> Fix: If user specifies either CLDR or COMPAT via System Property,
>>>>> FALLBACK should not be implicitly included.
>>>>>
>>>>> It is meant to be included when no ResourceBundleBased Adapters are
>>>>> available.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Rachna
>>>>>
>>>
>>
>



More information about the i18n-dev mailing list