<i18n dev> RFR: 8257964: Broken Calendar#getMinimalDaysInFirstWeek with java.locale.providers=HOST

Joe Wang joehw at openjdk.java.net
Fri Dec 11 00:28:59 UTC 2020


On Thu, 10 Dec 2020 21:12:29 GMT, Naoto Sato <naoto at openjdk.org> wrote:

> Hello,
> 
> Please review the changes to the subject issue. getMinimalDaysInFirstWeek() for Windows has been implemented to suffice the bug claim.

Looks good to me. Some minor comments below.

src/java.base/windows/classes/sun/util/locale/provider/HostLocaleProviderAdapterImpl.java line 78:

> 76:     // CalendarData value types
> 77:     private static final int CD_FIRSTDAYOFWEEK = 0;
> 78:     private static final int CD_MINIMALDAYSINFIRSTWEEK = 1;

Do we want to keep the naming consistent, doing the same change to, e.g. the corresponding macosx impl?

src/java.base/windows/classes/sun/util/locale/provider/HostLocaleProviderAdapterImpl.java line 373:

> 371:                         CD_FIRSTWEEKOFYEAR);
> 372:                 return switch (firstWeek) {
> 373:                     case 1 -> 7;

Would it be good to use constants or enum instead of literal, or maybe at least a note for the case numbers.

test/jdk/java/util/Locale/LocaleProvidersRun.java line 177:

> 175: 
> 176:         //testing 8257964 fix. (macOS/Windows only)
> 177:         testRun("HOST", "bug8257964Test", "", "", "");

This test runs only if the platform locale is en-GB. Do we know if the test system run tests on multiple locales? From the console output unfortunately, it's impossible to tell which specific tests were run

-------------

Marked as reviewed by joehw (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1741


More information about the i18n-dev mailing list