On Wed, 14 Apr 2021 18:01:10 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> Jaikiran Pai has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Update existing NarrowNamesTest to match expectations
>> - Naoto's review suggestion
>
> src/java.base/share/classes/sun/util/locale/provider/CalendarNameProviderImpl.java
> line 193:
>
>> 191: }
>> 192: if (field == AM_PM && !javatime && i > PM) {
>> 193: // when dealing with calendar fields,
>> don't set AM_PM field value
>
> Make it explicit that this block only serves for `java.util.Calendar`, not
> `java.time.format.DateTimeFormatter(Builder)`.
Hello Naoto, I've now updated the PR to be more explicit in this code comment.
> test/jdk/java/util/Calendar/CalendarDisplayNamesTest.java line 53:
>
>> 51: for (final int style : styles) {
>> 52: final Calendar cal = Calendar.getInstance();
>> 53: cal.setLenient(false);
>
> Don't think leniency is relevant here.
Removed in the latest update of this PR. This was a leftover from my
experimental testing.
> test/jdk/java/util/Calendar/NarrowNamesTest.java line 119:
>
>> 117: expectedFieldValues.put("a", Calendar.AM);
>> 118: expectedFieldValues.put("p", Calendar.PM);
>> 119: testAM_PM(US, ALL_STYLES, expectedFieldValues);
>
> I believe this can be reverted to the original, as the original code compares
> the exact map (including the length of the map). , i.e.,
>
> testMap(US, AM_PM, ALL_STYLES,
> "AM", "PM",
> RESET_INDEX,
> "a", "p");
Done. Reverted this specific part to what it was originally. Test continues to
pass with this change and the latest PR update.
-------------
PR: https://git.openjdk.java.net/jdk/pull/3463
