On 8/22/14 11:46 AM, Naoto Sato wrote:
http://cr.openjdk.java.net/~naoto/8038436/webrev.3/
I skimmed on the patch and have a few initial comment/questions.
JREENLocaleDataMetaInfo
JRENonENLocaleDataMetaInfo
- are the lists of locale names generated previously?
The long lines need to be broken up. Perhaps defining them
as string constants and grouped them per language (or some
fashion) would make it easier to read and maintain.
It would be good to define string constants for the en-* locale
names that are shared by FormatData, CurrencyNames, AvailbleLocales.
The @since 1.6 is not updated and not needed.
I wonder ifavailableLanguageTags andgetSupportedLocaleString
should return a list or an array of String (see comment below).
There are two provider implementations for
sun.util.locale.provider.LocaleDataMetaInfo and two service config
files as you want to put them in cldrdata.jar and localedata.jar.
I wonder if you want to merge them into a single service config
for now until you decide to separate them into separate modules
in the future.
You could simply put it under
src/jdk.localedata/share/META-INF/services/sun.util.locale.provider.LocaleDataMetaInfo
sun/util/cldr/CLDRLocaleProviderAdapter.java
line 67: why do you silently ignore any exception? In that
case, it will throw UOE in line 71. Is UOE caught somewhere?
Assume there is a good reason to ignore the exception,
can you add comment to explain it?
sun/util/locale/provider/JRELocaleProviderAdapter.java
line 391: what exceptions do you expect to be caught here?
Do you have a test case for this to demonstrate why you
have to ignore the exception?
LocaleDataMetaInfo.availableLanguageTags returns a string
of space-separated names. Would it be better to return
a List<String> or String[]
When you have an image build, it'd be useful to test without
cldrdata.jar and localedata.jar from the extension directory
and run the tests to use the default EN locale.
thanks
Mandy
