On Fri, 8 May 2026 10:10:02 GMT, Mikhail Yankelevich <[email protected]> 
wrote:

>> Adding new methods to `X509Certificate` to return `Instant` instead of 
>> `Date` as well as moving away from `Date` in internal packages wherever 
>> possible.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Mikhail Yankelevich has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   comments

Last few comments on the API. You can finalize your CSR after resolving them.

src/java.base/share/classes/java/security/cert/X509Certificate.java line 182:

> 180:      * @implSpec
> 181:      * The default implementation converts the specified {@code Instant} 
> to
> 182:      * {@code Date} and calls {@code checkValidity(date)}.

I feel a tiny uneasy at the `date` name in `checkValidity(data)` since it looks 
like a variable name but has not appeared earlier. On the other hand, I don't 
suggest using a `{@link}` because no other `@implSpec` (including your previous 
work on `KeyStore`) uses a link. I would suggest something like

    * The default implementation converts the specified {@code Instant} to
    * a {@code Date} and calls {@code checkValidity(Date)} with it.

src/java.base/share/classes/java/security/cert/X509Certificate.java line 191:

> 189:      * @throws    CertificateNotYetValidException if the certificate is 
> not
> 190:      * yet valid with respect to {@code Instant} supplied.
> 191:      * @throws    NullPointerException if supplied is {@code Instant} is 
> null.

Wording issue. Also, just use "instant".

src/java.base/share/classes/java/security/cert/X509Certificate.java line 388:

> 386:      * The default implementation calls {@code getNotAfter()}
> 387:      * and returns the output as an {@code Instant} value.
> 388:      * The {@code Date} returned by {@code getNotAfter()} should not be 
> null.

Remove the line above like in `getNotBeforeInstant`.

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

PR Review: https://git.openjdk.org/jdk/pull/30047#pullrequestreview-4252332707
PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r3208958169
PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r3208747721
PR Review Comment: https://git.openjdk.org/jdk/pull/30047#discussion_r3208759009

Reply via email to