On Fri, 11 Dec 2020 00:15:49 GMT, Joe Wang <jo...@openjdk.org> wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Added comment for LOCALE_IFIRSTWEEKOFYEAR Windows return values
>
> 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?
The constants are for native methods, which differ between macOS and Windows.
Thus I thought it would be clearer to align the name with Windows' constants.
> 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.
Could be better by making them as enums, but there are other locations
similarly using ints. I added extra comments to explain those ints for better
readability.
> 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
Could be, but it is enough to test one locale to demonstrate platform settings
are used for minimal days in first week. Each test case name can be found in
the command line to the java launcher which is logged in the .jtr file.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1741
