[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-i18n-dev
Subject:    Re: <i18n dev> RFR: 8225641: Calendar.roll(int field) does not work correctly for WEEK_OF_YEAR [v7]
From:       Naoto Sato <naoto () openjdk ! org>
Date:       2023-03-31 17:26:19
Message-ID: oSEEUpxVsKv8eojoP27sEgeA9vECA29C6PC-XIxTmSQ=.f582c67c-f51a-432f-8a5e-02ed553bfa1f () github ! com
[Download RAW message or body]

On Fri, 31 Mar 2023 16:39:17 GMT, Justin Lu <jlu@openjdk.org> wrote:

> > This PR fixes the bug which occurred when `Calendar.roll(WEEK_OF_YEAR)` rolled \
> > into a minimal first week with an invalid `WEEK_OF_YEAR` and `DAY_OF_WEEK` combo. \
> >  For example, Rolling _Monday, 30 December 2019_ by 1 week produced _Monday, 31 \
> > December 2018_, which is incorrect. This is because `WEEK_OF_YEAR` is rolled from \
> > 52 to 1, and the original `DAY_OF_WEEK` is 1. However, there is no Monday in week \
> > 1 of 2019. This is exposed when a future method calls `Calendar.complete()`, \
> > which eventually calculates a `fixedDate` with the invalid `WEEK_OF_YEAR` and \
> > `DAY_OF_WEEK` combo. 
> > To prevent this, a check is added for rolls into week 1, which determines if the \
> > first week is minimal. If it is indeed minimal, then it is checked if  \
> > `DAY_OF_WEEK` exists in that week, if not, `WEEK_OF_YEAR` must be incremented by \
> > one. 
> > After the fix, Rolling _Monday, 30 December 2019_ by 1 week produces _Monday, 7 \
> > January 2019_
> 
> Justin Lu has updated the pull request incrementally with one additional commit \
> since the last revision: 
> getFirstDayInWeek() is well checked, so need for exception in dayInMinWeek

LGTM

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

Marked as reviewed by naoto (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13031#pullrequestreview-1367321853


[prev in list] [next in list] [prev in thread] [next in thread]