Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v6]
On Wed, 21 Apr 2021 09:06:45 GMT, Daniel Fuchs wrote:
>> Patrick Concannon has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Updated single letter pattern variable names
>
> src/java.base/share/classes/java/time/Duration.java line 723:
>
>> 721: }
>> 722: if (unit instanceof ChronoUnit u) {
>> 723: switch (u) {
>
> Maybe `u` could be replaced with `chronoUnit` here too
Variable name updated as suggested in 3dc1e07
-
PR: https://git.openjdk.java.net/jdk/pull/3170
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v6]
On Wed, 24 Mar 2021 11:06:38 GMT, Rémi Forax
wrote:
>> Patrick Concannon has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Updated single letter pattern variable names
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
> line 168:
>
>> 166: private static final TemporalQuery QUERY_REGION_ONLY =
>> (temporal) -> {
>> 167: ZoneId zone = temporal.query(TemporalQueries.zoneId());
>> 168: return (zone != null && (!(zone instanceof ZoneOffset)) ? zone
>> : null);
>
> i find this code hard to read
> `return (zone != null && (!(zone instanceof ZoneOffset))) ? zone : null;`
> seems better`
Updated in 647bd6b as suggested by Michael Kuhlmann
-
PR: https://git.openjdk.java.net/jdk/pull/3170
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v6]
On Tue, 20 Apr 2021 17:46:38 GMT, Patrick Concannon
wrote:
>> Hi,
>>
>> Could someone please review my code for updating the code in the `java.time`
>> package to make use of the `instanceof` pattern variable?
>>
>> Kind regards,
>> Patrick
>
> Patrick Concannon has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Updated single letter pattern variable names
src/java.base/share/classes/java/time/Duration.java line 723:
> 721: }
> 722: if (unit instanceof ChronoUnit u) {
> 723: switch (u) {
Maybe `u` could be replaced with `chronoUnit` here too
-
PR: https://git.openjdk.java.net/jdk/pull/3170
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v6]
On Tue, 20 Apr 2021 17:46:38 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.time` >> package to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with one > additional commit since the last revision: > > Updated single letter pattern variable names Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3170
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v6]
On Tue, 20 Apr 2021 17:46:38 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.time` >> package to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with one > additional commit since the last revision: > > Updated single letter pattern variable names Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3170
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v6]
On Wed, 24 Mar 2021 10:57:11 GMT, Rémi Forax
wrote:
>> Patrick Concannon has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Updated single letter pattern variable names
>
> src/java.base/share/classes/java/time/LocalDateTime.java line 1686:
>
>> 1684: public long until(Temporal endExclusive, TemporalUnit unit) {
>> 1685: LocalDateTime end = LocalDateTime.from(endExclusive);
>> 1686: if (unit instanceof ChronoUnit u) {
>
> `chronoUnit` is perhaps a better variable name than `u`
Thanks for your comments, @forax, and apologizes for the delay in getting back
to you. I was waiting for the boot JDK version to be updated to 16. Certain
files changed in the PR are shared between the build tool and the JDK runtime,
and were causing build issues.
I've addressed the changes you suggested, and you can find them in commit
647bd6b
-
PR: https://git.openjdk.java.net/jdk/pull/3170
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v6]
> Hi, > > Could someone please review my code for updating the code in the `java.time` > package to make use of the `instanceof` pattern variable? > > Kind regards, > Patrick Patrick Concannon has updated the pull request incrementally with one additional commit since the last revision: Updated single letter pattern variable names - Changes: - all: https://git.openjdk.java.net/jdk/pull/3170/files - new: https://git.openjdk.java.net/jdk/pull/3170/files/2dca4a09..647bd6b1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3170=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3170=04-05 Stats: 51 lines in 14 files changed: 0 ins; 0 del; 51 mod Patch: https://git.openjdk.java.net/jdk/pull/3170.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3170/head:pull/3170 PR: https://git.openjdk.java.net/jdk/pull/3170
