Integrated: 8255226: (tz) Upgrade time-zone data to tzdata2020d
On Mon, 2 Nov 2020 16:29:07 GMT, Kiran Sidhartha Ravikumar wrote: > Hi Guys, > > Please review the integration of tzdata2020d to JDK. > > Details regarding the change can be viewed at - > https://mm.icann.org/pipermail/tz-announce/2020-October/62.html > Bug: https://bugs.openjdk.java.net/browse/JDK-8255226 > > TestZoneInfo310.java test failure is addressed along with it. The last rule > affects "Asia/Gaza" and "Asia/Hebron" and therefore excluded from the test. > > Regression Tests pass along with JCK. > > Please let me know if the changes are good to push. > > Thanks, > Kiran This pull request has now been integrated. Changeset: b65ff60a Author:Kiran Sidhartha Ravikumar Committer: Sean Coffey URL: https://git.openjdk.java.net/jdk/commit/b65ff60a Stats: 61 lines in 4 files changed: 38 ins; 2 del; 21 mod 8255226: (tz) Upgrade time-zone data to tzdata2020d Reviewed-by: naoto - PR: https://git.openjdk.java.net/jdk/pull/1012
Re: RFR: 8247781: Day periods support [v3]
On Fri, 30 Oct 2020 22:03:08 GMT, Naoto Sato wrote: >> Hi, >> >> Please review the changes for the subject issue. This is to enhance the >> java.time package to support day periods, such as "in the morning", defined >> in CLDR. It will add a new pattern character 'B' and its supporting builder >> method. The motivation and its spec are in this CSR: >> >> https://bugs.openjdk.java.net/browse/JDK-8254629 >> >> Naoto > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed exception messages. Marked as reviewed by rriggs (Reviewer). src/java.base/share/classes/java/time/format/Parsed.java line 180: > 178: cloned.chrono = this.chrono; > 179: cloned.leapSecond = this.leapSecond; > 180: cloned.dayPeriod= this.dayPeriod; Add space before "=". - PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8247781: Day periods support [v6]
On Wed, 4 Nov 2020 22:13:25 GMT, Naoto Sato wrote:
>> Hi,
>>
>> Please review the changes for the subject issue. This is to enhance the
>> java.time package to support day periods, such as "in the morning", defined
>> in CLDR. It will add a new pattern character 'B' and its supporting builder
>> method. The motivation and its spec are in this CSR:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8254629
>>
>> Naoto
>
> Naoto Sato has updated the pull request with a new target base due to a merge
> or a rebase. The incremental webrev excludes the unrelated changes brought in
> by the merge/rebase. The pull request contains ten additional commits since
> the last revision:
>
> - Merge branch 'master' into dayperiod
> - Change based on CSR change.
> - Fixed TCK test failures, added the new pattern char description in
> DateTimeFormatter
> - Addressing https://github.com/openjdk/jdk/pull/938#discussion_r516147298
> - Addressing https://github.com/openjdk/jdk/pull/938#discussion_r516123190
> - Addressing https://github.com/openjdk/jdk/pull/938#discussion_r516138455
> - Fixed exception messages.
> - Addressed the following comments:
>- https://github.com/openjdk/jdk/pull/938#discussion_r515003422
>- https://github.com/openjdk/jdk/pull/938#discussion_r515005296
>- https://github.com/openjdk/jdk/pull/938#discussion_r515008862
>- https://github.com/openjdk/jdk/pull/938#discussion_r515030268
>- https://github.com/openjdk/jdk/pull/938#discussion_r515030880
>- https://github.com/openjdk/jdk/pull/938#discussion_r515032002
>- https://github.com/openjdk/jdk/pull/938#discussion_r515036803
>- https://github.com/openjdk/jdk/pull/938#discussion_r515037626
>- https://github.com/openjdk/jdk/pull/938#discussion_r515038069
>- https://github.com/openjdk/jdk/pull/938#discussion_r515039056
> - 8247781: Day periods support
src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line
1479:
> 1477: * for it in the formatter locale is from 21:00 to 06:00, then
> {@code HOUR_OF_DAY}
> 1478: * is set to '1' and {@code MINUTE_OF_HOUR} set to '30'. If {@code
> AMPM_OF_DAY} exists
> 1479: * and no {@code HOUR_OF_DAY} is resolved, the parsed day period
> takes the precedence.
"the precedence" -> "precedence"
-
PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8247781: Day periods support [v7]
> Hi, > > Please review the changes for the subject issue. This is to enhance the > java.time package to support day periods, such as "in the morning", defined > in CLDR. It will add a new pattern character 'B' and its supporting builder > method. The motivation and its spec are in this CSR: > > https://bugs.openjdk.java.net/browse/JDK-8254629 > > Naoto Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Fixed typo/grammatical error. - Changes: - all: https://git.openjdk.java.net/jdk/pull/938/files - new: https://git.openjdk.java.net/jdk/pull/938/files/4aa5b197..b0649899 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=938&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=938&range=05-06 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/938.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/938/head:pull/938 PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8247781: Day periods support [v7]
On Thu, 5 Nov 2020 16:07:30 GMT, Roger Riggs wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixed typo/grammatical error.
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
> line 1479:
>
>> 1477: * for it in the formatter locale is from 21:00 to 06:00, then
>> {@code HOUR_OF_DAY}
>> 1478: * is set to '1' and {@code MINUTE_OF_HOUR} set to '30'. If {@code
>> AMPM_OF_DAY} exists
>> 1479: * and no {@code HOUR_OF_DAY} is resolved, the parsed day period
>> takes the precedence.
>
> "the precedence" -> "precedence"
Fixed.
-
PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8247781: Day periods support [v3]
On Mon, 2 Nov 2020 19:20:10 GMT, Roger Riggs wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fixed exception messages. > > src/java.base/share/classes/java/time/format/Parsed.java line 180: > >> 178: cloned.chrono = this.chrono; >> 179: cloned.leapSecond = this.leapSecond; >> 180: cloned.dayPeriod= this.dayPeriod; > > Add space before "=". Fixed. - PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8247781: Day periods support [v7]
On Thu, 5 Nov 2020 17:12:11 GMT, Naoto Sato wrote: >> Hi, >> >> Please review the changes for the subject issue. This is to enhance the >> java.time package to support day periods, such as "in the morning", defined >> in CLDR. It will add a new pattern character 'B' and its supporting builder >> method. The motivation and its spec are in this CSR: >> >> https://bugs.openjdk.java.net/browse/JDK-8254629 >> >> Naoto > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed typo/grammatical error. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8247781: Day periods support [v7]
On Thu, 5 Nov 2020 17:12:11 GMT, Naoto Sato wrote:
>> Hi,
>>
>> Please review the changes for the subject issue. This is to enhance the
>> java.time package to support day periods, such as "in the morning", defined
>> in CLDR. It will add a new pattern character 'B' and its supporting builder
>> method. The motivation and its spec are in this CSR:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8254629
>>
>> Naoto
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixed typo/grammatical error.
test/jdk/java/time/tck/java/time/format/TCKDateTimeParseResolver.java line 858:
> 856: return new Object[][]{
> 857: {STRICT, 0, LocalTime.of(6, 0), 0},
> 858: {STRICT, 1, LocalTime.of(18, 0), 1},
As mentioned in my other comment, this seems odd in STRICT mode.
src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line
5055:
> 5053: @Override
> 5054: public boolean format(DateTimePrintContext context,
> StringBuilder buf) {
> 5055: Long value = context.getValue(MINUTE_OF_DAY);
This does not match the spec: " During formatting, the day period is obtained
from {@code HOUR_OF_DAY}, and optionally {@code MINUTE_OF_HOUR} if exist"
It is possible and legal to create a Temporal that returns `HOUR_OF_DAY` and
`MINUTE_OF_HOUR` but not `MINUTE_OF_DAY`. As such, this method must be changed
to follow the spec.
-
In addition, it is possible for `HOUR_OF_DAY` and `MINUTE_OF_HOUR` to be
outside their normal bounds. The right behaviour would be to combine the two
fields within this method, and then use mod to get the value into the range 0
to 1440 before calling `dayPeriod.include`. (While the fall back behaviour
below does cover this, it would be better to do what I propose here.)
An example of this is a `TransportTime` class where the day runs from 03:00 to
27:00 each day (because trains run after midnight for no extra cost to the
passenger, and it is more convenient for the operator to treat the date that
way). A `TransportTime` of 26:30 should still resolve to "night1" rather than
fall back to "am".
-
PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8247781: Day periods support [v7]
On Mon, 2 Nov 2020 23:21:22 GMT, Naoto Sato wrote: >> Pulling on this a little more. >> >> As the PR stands, it seems that if the user passes in text with just a >> day-period of "AM" they get a `LocalTime` of 06:00 but if they pass in >> `AMPM_OF_DAY` of "AM" the get no `LocalTime` in the result. Is that right? >> If so, I don't think this is sustainable. >> >> Thats why I think `AMPM_OF_DAY` will have to be resolved to a dayPeriod of >> "am" or "pm". If dayPeriod is more precise than `AMPM_OF_DAY`, then >> dayPeriod can silently take precedence > > I implemented what you suggested here in the latest PR, but that would be a > behavioral change which requires a CSR, as "AM" would be resolved to 06:00 > which was not before. Do you think it would be acceptable? If so, I will > reopen the CSR and describe the change. (In fact some TCK failed with this > impl) I find the whole "half way between the start and end" behaviour of day periods odd anyway, but if it is to be supported then it should be consistent as you've implemented. Another option I should have thought of earlier would be to simply not support the "half way between the start and end" behaviour of LDML in either dayPeriod or AM/PM. But since LDML defines it, you've implemented it, and it isn't overly harmful I think its OK to leave it in. Would it be possible for STRICT mode to not have the "half way between the start and end" behaviour? - PR: https://git.openjdk.java.net/jdk/pull/938
Re: RFR: 8247781: Day periods support [v7]
On Thu, 5 Nov 2020 23:25:38 GMT, Stephen Colebourne
wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixed typo/grammatical error.
>
> test/jdk/java/time/tck/java/time/format/TCKDateTimeParseResolver.java line
> 858:
>
>> 856: return new Object[][]{
>> 857: {STRICT, 0, LocalTime.of(6, 0), 0},
>> 858: {STRICT, 1, LocalTime.of(18, 0), 1},
>
> As mentioned in my other comment, this seems odd in STRICT mode.
Did you mean in STRICT mode, HOUR_OF_AMPM should default to 0, and to 6 in
SMART/LENIENT modes?
-
PR: https://git.openjdk.java.net/jdk/pull/938
