<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