On Tue, 14 Jul 2026 20:30:44 GMT, Justin Lu <[email protected]> wrote:
>> Sholto has updated the pull request incrementally with one additional commit >> since the last revision: >> >> 8272194: only use created calendar for BC check in LocalDate/LocalDateTime >> conversion > > src/java.sql/share/classes/java/sql/Timestamp.java line 567: > >> 565: getMinutes(), >> 566: getSeconds(), >> 567: getNanos()); // We have to use nanos >> here as the calendar does not support nano precision > > This comment is stale and can be removed. Good spotting. I've removed that now. > test/jdk/java/sql/test/sql/DateTests.java line 334: > >> 332: * Ensures that the fix for 8272194 has not regressed. >> 333: */ >> 334: @ParameterizedTest(autoCloseArguments = false) > > I understand you are following the existing test convention, but I would > remove the `autoCloseArguments = false` here and in the other occurrences > since the parameter does not implement `AutoCloseable`, so it's just noise. > (It is added to those other tests even though not needed as part of an > earlier migration to conservatively match TestNG semantics.) That is good to know. I have removed them from the tests I have added, but I have left the existing uses alone. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31808#discussion_r3583006961 PR Review Comment: https://git.openjdk.org/jdk/pull/31808#discussion_r3583015710
