Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v3]

2024-04-17 Thread Jaikiran Pai
On Wed, 10 Apr 2024 16:17:32 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which proposes to fix the issue >> noted in https://bugs.openjdk.org/browse/JDK-8212895? >> >> As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is >> initialized to have a

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v3]

2024-04-16 Thread Roger Riggs
On Wed, 10 Apr 2024 16:17:32 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which proposes to fix the issue >> noted in https://bugs.openjdk.org/browse/JDK-8212895? >> >> As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is >> initialized to have a

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v3]

2024-04-16 Thread Naoto Sato
On Tue, 16 Apr 2024 06:08:00 GMT, Jaikiran Pai wrote: > Naoto, Roger, should we consider a release note for this change or is the CSR > itself enough? I think CSR is enough, as users' chance of encountering any issue is very slim. My $0.02 - PR Comment:

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v3]

2024-04-16 Thread Jaikiran Pai
On Wed, 10 Apr 2024 16:17:32 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which proposes to fix the issue >> noted in https://bugs.openjdk.org/browse/JDK-8212895? >> >> As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is >> initialized to have a

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v3]

2024-04-11 Thread Jaikiran Pai
On Wed, 10 Apr 2024 16:17:32 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which proposes to fix the issue >> noted in https://bugs.openjdk.org/browse/JDK-8212895? >> >> As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is >> initialized to have a

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v3]

2024-04-10 Thread Naoto Sato
On Wed, 10 Apr 2024 16:17:32 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which proposes to fix the issue >> noted in https://bugs.openjdk.org/browse/JDK-8212895? >> >> As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is >> initialized to have a

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v2]

2024-04-10 Thread Jaikiran Pai
On Wed, 10 Apr 2024 12:27:33 GMT, Naoto Sato wrote: >> Hello Naoto, I had used `InstantSeconds` to keep it consistent with how a >> similar doc is used for the `EPOCH_DAY` field. Let me know if you still >> prefer this to be `INSTANT_SECONDS` and I will update it. > > With the @code tag, I

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v3]

2024-04-10 Thread Jaikiran Pai
> Can I please get a review of this change which proposes to fix the issue > noted in https://bugs.openjdk.org/browse/JDK-8212895? > > As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is > initialized to have a minimum and maximum values of `Long.MIN_VALUE` and >

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v2]

2024-04-10 Thread Jaikiran Pai
On Tue, 9 Apr 2024 08:37:29 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which proposes to fix the issue >> noted in https://bugs.openjdk.org/browse/JDK-8212895? >> >> As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is >> initialized to have a

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v2]

2024-04-10 Thread Naoto Sato
On Wed, 10 Apr 2024 01:31:36 GMT, Jaikiran Pai wrote: >> src/java.base/share/classes/java/time/temporal/ChronoField.java line 590: >> >>> 588: * This is necessary to ensure interoperation between calendars. >>> 589: * >>> 590: * Range of {@code InstantSeconds} is between {@link

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v2]

2024-04-09 Thread Jaikiran Pai
On Tue, 9 Apr 2024 16:29:07 GMT, Naoto Sato wrote: >> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Naoto's suggestion - use Instant.MIN and Instant.MAX instead of hardcoded >> values > >

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v2]

2024-04-09 Thread Naoto Sato
On Tue, 9 Apr 2024 08:37:29 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which proposes to fix the issue >> noted in https://bugs.openjdk.org/browse/JDK-8212895? >> >> As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is >> initialized to have a

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v2]

2024-04-09 Thread Roger Riggs
On Tue, 9 Apr 2024 08:34:39 GMT, Jaikiran Pai wrote: >> Should `INSTANT_SECONDS("InstantSeconds", SECONDS, FOREVER, >> ValueRange.of(Instant.MIN.getEpochSecond(), Instant.MAX.getEpochSecond())), >> ` work? > > Hello Naoto, that's a very good point. I was too caught up with the constant >

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v2]

2024-04-09 Thread Roger Riggs
On Tue, 9 Apr 2024 08:37:29 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which proposes to fix the issue >> noted in https://bugs.openjdk.org/browse/JDK-8212895? >> >> As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is >> initialized to have a

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v2]

2024-04-09 Thread Jaikiran Pai
On Tue, 9 Apr 2024 01:34:56 GMT, Naoto Sato wrote: >> Hello Roger, >> >>> The code should reference the constants in Instant.java (though may need to >>> make them package private.) >> >> The `ChronoField` class and the `Instant` class reside in separate packages, >> so making these two

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v2]

2024-04-09 Thread Jaikiran Pai
> Can I please get a review of this change which proposes to fix the issue > noted in https://bugs.openjdk.org/browse/JDK-8212895? > > As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is > initialized to have a minimum and maximum values of `Long.MIN_VALUE` and >

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant

2024-04-08 Thread Naoto Sato
On Tue, 9 Apr 2024 00:52:43 GMT, Jaikiran Pai wrote: >> The code should reference the constants in Instant.java (though may need to >> make them package private.) >> >> The javadoc can/should reference Instant.MIN and Instant.MAX (as the test >> does). > > Hello Roger, > >> The code should

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant

2024-04-08 Thread Jaikiran Pai
On Mon, 8 Apr 2024 18:16:04 GMT, Roger Riggs wrote: >> src/java.base/share/classes/java/time/temporal/ChronoField.java line 590: >> >>> 588: * This is necessary to ensure interoperation between calendars. >>> 589: */ >>> 590: // ValueRange matches the min and max epoch second

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant

2024-04-08 Thread Roger Riggs
On Mon, 8 Apr 2024 18:00:02 GMT, Naoto Sato wrote: >> Can I please get a review of this change which proposes to fix the issue >> noted in https://bugs.openjdk.org/browse/JDK-8212895? >> >> As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is >> initialized to have a minimum

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant

2024-04-08 Thread Naoto Sato
On Mon, 8 Apr 2024 09:52:39 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which proposes to fix the issue > noted in https://bugs.openjdk.org/browse/JDK-8212895? > > As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is > initialized to have a minimum

Re: RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant

2024-04-08 Thread Jaikiran Pai
On Mon, 8 Apr 2024 09:52:39 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which proposes to fix the issue > noted in https://bugs.openjdk.org/browse/JDK-8212895? > > As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is > initialized to have a minimum

RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant

2024-04-08 Thread Jaikiran Pai
Can I please get a review of this change which proposes to fix the issue noted in https://bugs.openjdk.org/browse/JDK-8212895? As noted in that issue, the `ChronoField.INSTANT_SECONDS` currently is initialized to have a minimum and maximum values of `Long.MIN_VALUE` and `LONG.MAX_VALUE`