Review request for IMF classes and Locale related classes

[Stuart Marks]

Looks good! Please proceed with the push.

thanks.

s'marks


On 12/5/11 11:01 AM, Naoto Sato wrote:
> OK here is the revised one:
>
> http://cr.openjdk.java.net/~naoto/7117469/webrev.02/
>
> I hope this is the final :-)
>
> Naoto
>
> On 12/2/11 5:41 PM, Stuart Marks wrote:
>> The changes you made are good, thanks.
>>
>> I did figure out how to deal with the static initializer in
>> AllAvailableLocales:
>>
>> @SuppressWarnings({"unchecked"})
>> Class<? extends LocaleServiceProvider>[] providerClasses =
>> (Class<? extends LocaleServiceProvider>[]) new Class<?>[] {
>> java.text.spi.BreakIteratorProvider.class,
>> ...
>> };
>>
>> This is a bit uglier but it does reduce the scope of warnings suppression.
>>
>> I think you might be able to get rid of the wildcards, too:
>>
>> @SuppressWarnings({"unchecked"})
>> Class<LocaleServiceProvider>[] providerClasses =
>> (Class<LocaleServiceProvider>[]) new Class<?>[] {
>> java.text.spi.BreakIteratorProvider.class,
>> ...
>> };
>> ...
>> for (Class<LocaleServiceProvider> providerClass : providerClasses) {
>> ...
>> }
>>
>> I'm not entirely sure why this works, but it seems to compile for me
>> without warnings or errors....
>>
>> s'marks
>>
>>
>>
>> On 12/2/11 4:00 PM, Naoto Sato wrote:
>>> Hi Stuart,
>>>
>>> Thank you for your comments. Here is the updated webrev reflecting your
>>> suggestions. Please review:
>>>
>>> http://cr.openjdk.java.net/~naoto/7117469/webrev.01/
>>>
>>> Naoto
>>>
>>>
>>> On 12/2/11 3:32 PM, Stuart Marks wrote:
>>>> Hi Naoto,
>>>>
>>>> A couple comments.
>>>>
>>>> java/util/Currency.java --
>>>>
>>>> The @SuppressWarnings covers the entire method. We're trying to use
>>>> @SuppressWarnings with as narrow a scope as possible. Sometimes it's
>>>> helpful to create a local variable declaration for this purpose; perhaps
>>>> something like this will help:
>>>>
>>>> @SuppressedWarnings("unchecked")
>>>> Set<Currency> result = (Set<Currency>) available.clone();
>>>> return result;
>>>>
>>>> You can probably do something similar in
>>>> sun/util/LocaleServiceProviderPool.java in the getLocalizedObjectImpl()
>>>> method.
>>>>
>>>> The suppression of warnings on the static AllAvailableLocales class is a
>>>> bit of a puzzle. Maybe it's OK to leave the suppression for that entire
>>>> class.
>>>>
>>>> s'marks
>>>>
>>>>
>>>> On 12/2/11 2:24 PM, Masayoshi Okutsu wrote:
>>>>> Looks good to me.
>>>>>
>>>>> Thanks,
>>>>> Masayoshi
>>>>>
>>>>> On 2011/12/02 10:56, Naoto Sato wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Could you please review these two changesets for the WCD? One is for
>>>>>> classes
>>>>>> that belongs to input method framework:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~naoto/7117465/webrev.00/
>>>>>>
>>>>>> and the other is for (some of the) i18n related *.util.* classes:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~naoto/7117469/webrev.00/
>>>>>>
>>>>>> Thanks!
>>>>>> Naoto
>>>
>