On Tue, 10 May 2022 13:12:10 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fixed offsets in milliseconds, added test variations, refined Custom ID >> definitions > > src/java.base/share/classes/java/util/TimeZone.java line 109: > >> 107: * <blockquote><pre> >> 108: * <a id="NormalizedCustomID"><i>NormalizedCustomID:</i></a> >> 109: * {@code GMT} <i>Sign</i> <i>TwoDigitHours</i> {@code :} >> <i>Minutes</i> [<i>Seconds</i>] > > Hello Naoto, > > Should this instead be: `... <i>Minutes</i> [{@code :} <i>Seconds</i>]` - > i.e. should it have the `:` literal if seconds are present in the custom > timezone id? The colon is included in `Seconds` part below. I changed the part name to `ColonSeconds` to make it clearer. > src/java.base/share/classes/java/util/TimeZone.java line 543: > >> 541: return new ZoneInfo(totalSecs == 0 ? "UTC" : GMT_ID + tzid, >> totalSecs); >> 542: } else { >> 543: return getTimeZone(tzid, true); > > Before the change in this PR, we used to prefix `GMT` to (non-custom timezone > ids) if the timezone id returned by `ZoneId#getId()` started with the `+` or > `-` sign, before calling `getTimeZone(modifiedTzid, true)`. > With this change, for `ZoneId`s that aren't `ZoneOffset` instance, we now > call `getTimeZone(originalTzid, true)`, without first checking/prefixing the > id with `GMT`. Is that an intentional change and would that potentially cause > `getTimeZone(String, boolean)` to return a different result? Yes, it is intentional. The `Time-zone IDs` section in the `ZoneId` class description is clear that zone id starting with "+/-" is a `ZoneOffset` instance. Other ZoneIds should have offsets with prefix or region-based ids. ------------- PR: https://git.openjdk.java.net/jdk/pull/8606