On Thu, 18 Sep 2025 20:41:58 GMT, Justin Lu <[email protected]> wrote:
>> Please review this PR which addresses an edge case for >> `GregorianCalendar.roll(int, int)` when the rolled amount would cause the >> hour to remain the same as before the call. After this change, the expected >> hour is returned. That is, rolling a full cycle for HOUR (12 hours) and >> HOUR_OF_DAY (24 hours) should keep the hour the same as before the call. >> >> For example, a calendar with HOUR_OF_DAY == 15, >> >> >> cal.roll(Calendar.HOUR_OF_DAY, 23); >> cal.get(Calendar.HOUR_OF_DAY); // returns 14 >> >> >> cal.roll(Calendar.HOUR_OF_DAY, 24); >> // Incorrectly returns 16. A full cycle is expected to return the starting >> hour (15) >> cal.get(Calendar.HOUR_OF_DAY); >> >> >> cal.roll(Calendar.HOUR_OF_DAY, 25); >> cal.get(Calendar.HOUR_OF_DAY); // returns 16 > > Justin Lu has updated the pull request incrementally with two additional > commits since the last revision: > > - + - to - in comment > - DST test cases + compute expected value as method + improve err msg LGTM test/jdk/java/util/Calendar/RollHoursTest.java line 73: > 71: calendars.get(0).set(2005, 8, 20, 12, 10, 25); > 72: > 73: // Transition to daylight savings time (CST/CDT) --- Nit: I think it is usually called "daylight saving time" ------------- Marked as reviewed by naoto (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/27355#pullrequestreview-3242247888 PR Review Comment: https://git.openjdk.org/jdk/pull/27355#discussion_r2361302216
