<i18n dev> RFR: 8212895: ChronoField.INSTANT_SECONDS's range doesn't match the range of Instant [v2]

Jaikiran Pai jpai at openjdk.org
Tue Apr 9 08:37:29 UTC 2024


On Tue, 9 Apr 2024 01:34:56 GMT, Naoto Sato <naoto at openjdk.org> 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 fields in `Instant`, package private will not help. I will have to make them public, which I think probably isn't a good idea. Unless you think we should do it? There is one other place already in the JDK where we have copy/pasted these values `src/java.base/share/classes/java/nio/file/attribute/FileTime.java`, so maybe we can continue with this copy/paste here too?
>> 
>> As for the javadoc, after we decide about this field access detail, I'll update it accordingly to note that the values min and max range complies with the min and max epoch seconds supported by `Instant`.
>
> 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 values that it didn't occur to me that the `Instant.MIN` and `Instant.MAX` public fields could be used for this. I have followed your suggestion and updated the PR. I have also updated the javadoc to link to `Instant.MIN` and `Instant.MAX` as the supported epoch second range.

The test continues to pass with this change and fails (as expected) without the source change.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18674#discussion_r1557216492


More information about the i18n-dev mailing list