On Thu, 23 Jan 2025 02:49:49 GMT, Justin Lu <j...@openjdk.org> wrote:
> Please review this PR and CSR which add a pair of methods to > _java.util.TimeZone_ that return a stream of the available IDs. See the CSR > for the motivation. > > A number of existing tests are modified to use the new methods. See > _test/jdk/java/util/TimeZone/AvailableIDsTest.java_ which tests the new > methods. Looks good. Some files need copyright year bump src/java.base/share/classes/java/util/TimeZone.java line 652: > 650: * > 651: * @param rawOffset the given time zone GMT offset in milliseconds. > 652: * @return an array of IDs, where the time zone for that ID has array -> stream src/java.base/share/classes/sun/util/calendar/ZoneInfoFile.java line 96: > 94: // specify it. Keep the same behavior for better > 95: // compatibility. > 96: String[] list = ids.toArray(new String[0]); Why is this change? Since the array has correct size, no extra array allocation is needed? src/java.base/share/classes/sun/util/calendar/ZoneInfoFile.java line 122: > 120: return zoneIds() > 121: .filter(id -> getZoneInfo(id).getRawOffset() == > rawOffset) > 122: .sorted(); // Sort the IDs, see getZoneIds(int) This is interesting. one with `rawOffset` is soreted, but the other is not. It is inconsistent but probably we should align as you did here. ------------- PR Review: https://git.openjdk.org/jdk/pull/23251#pullrequestreview-2570586882 PR Review Comment: https://git.openjdk.org/jdk/pull/23251#discussion_r1927410357 PR Review Comment: https://git.openjdk.org/jdk/pull/23251#discussion_r1927429593 PR Review Comment: https://git.openjdk.org/jdk/pull/23251#discussion_r1927452374