Review request for IMF classes and Locale related classes (original) (raw)

Stuart Marks stuart.marks at oracle.com
Mon Dec 5 17:00:44 PST 2011

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[]) 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[] providerClasses = (Class[]) new Class<?>[] { java.text.spi.BreakIteratorProvider.class, ... }; ... for (Class 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 result = (Set) 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

More information about the jdk8-dev mailing list