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

Reply via email to