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 [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 >