Re: RFR: JDK-8326951 Missing @since Tags [v2]
On Tue, 19 Mar 2024 11:21:14 GMT, Nizar Benalla wrote:
>> I added `@since` tags for methods/constructors that do not match the
>> `@since` of the enclosing class.
>>
>> The `write` method already existed in `PrintStream` in earlier versions and
>> instances of it could always call this method, since it extends
>> `FilterOutputStream` [which has the
>> method](https://github.com/openjdk/jdk6/blob/3e49aa876353eaa215cde71eb21acc9b7f9872a0/jdk/src/share/classes/java/io/FilterOutputStream.java#L96).
>
> Nizar Benalla has updated the pull request incrementally with three
> additional commits since the last revision:
>
> - Revert "fix rest of private/public since tags"
>
>This reverts commit 2c04a9d8e773616b7b6239335d4e5eb955944ad1.
> - Revert "removed unnecessary @ since tags"
>
>This reverts commit 5da3f0d83d19393eeb3a9da68aac40dd999de660.
> - removed unnecessary @ since tags
src/java.base/share/classes/javax/crypto/interfaces/DHPublicKey.java line 68:
> 66: *
> 67: * @return {@inheritDoc java.security.AsymmetricKey}
> 68: * @since 22
Hello Nizar, are the removal of `@since` in this PR intentional? I haven't
checked all of them, but this specific removal appears incorrect, since this
method was indeed introduced in Java 22 as part of
https://bugs.openjdk.org/browse/JDK-8318096.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18055#discussion_r1526987579
Re: RFR: JDK-8326951 Missing @since Tags [v2]
On Mon, 18 Mar 2024 14:02:20 GMT, Jan Lahoda wrote:
>> Hello Jaikiran,
>> in jdk21 DHPPublicKey did have a
>> [getParams()](https://github.com/openjdk/jdk21/blob/890adb6410dab4606a4f26a942aed02fb2f55387/src/java.base/share/classes/com/sun/crypto/provider/DHPublicKey.java#L244)
>> method, so it is not new in jdk 22. It also existed [before
>> that](https://github.com/openjdk/jdk11/blob/37115c8ea4aff13a8148ee2b8832b20888a5d880/src/java.base/share/classes/com/sun/crypto/provider/DHPublicKey.java#L252)
>>
>> If you haven't looked at the other cases, I was about to group the changes
>> related to the Key interfaces in a separate PR if that's fine. let me know
>> what you think
>
> Hello,
>
> While the override of `getParams` in `DHPublicKey` was added, the `getParams`
> method has been inherited to `DHPublicKey` for a long time from `DHKey`.
> E.g., I can take this:
>
> import javax.crypto.interfaces.DHPublicKey;
>
> public class DhkeyTest {
>
> public static void main(DHPublicKey key) {
> System.err.println(key.getParams());
> }
>
> }
>
>
> and compile using JDK 8 without any compile-time errors. So, it would make
> more sense to me to not add the `@since` for it.
>
> I believe Nizar will separate the changes to the Key interfaces into a
> separate PR, so we can discuss in more detail there.
>
> Thanks!
Hello Jan, interesting. I hadn't noticed that
`javax.crypto.interfaces.DHPublicKey` already was exposing `getParams()` in
earlier versions because `javax.crypto.interfaces.DHPublicKey` extends from
`javax.crypto.interfaces.DHKey` which has the `getParams()` method.
> I believe Nizar will separate the changes to the Key interfaces into a
> separate PR, so we can discuss in more detail there.
That sounds fine to me.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18055#discussion_r1528641492
Re: RFR: JDK-8326951 Missing @since Tags [v2]
On Sun, 17 Mar 2024 01:20:17 GMT, Nizar Benalla wrote:
>> src/java.base/share/classes/javax/crypto/interfaces/DHPublicKey.java line 68:
>>
>>> 66: *
>>> 67: * @return {@inheritDoc java.security.AsymmetricKey}
>>> 68: * @since 22
>>
>> Hello Nizar, are the removal of `@since` in this PR intentional? I haven't
>> checked all of them, but this specific removal appears incorrect, since this
>> method was indeed introduced in Java 22 as part of
>> https://bugs.openjdk.org/browse/JDK-8318096.
>
> Hello Jaikiran,
> in jdk21 DHPPublicKey did have a
> [getParams()](https://github.com/openjdk/jdk21/blob/890adb6410dab4606a4f26a942aed02fb2f55387/src/java.base/share/classes/com/sun/crypto/provider/DHPublicKey.java#L244)
> method, so it is not new in jdk 22. It also existed [before
> that](https://github.com/openjdk/jdk11/blob/37115c8ea4aff13a8148ee2b8832b20888a5d880/src/java.base/share/classes/com/sun/crypto/provider/DHPublicKey.java#L252)
>
> If you haven't looked at the other cases, I was about to group the changes
> related to the Key interfaces in a separate PR if that's fine. let me know
> what you think
Hello,
While the override of `getParams` in `DHPublicKey` was added, the `getParams`
method has been inherited to `DHPublicKey` for a long time from `DHKey`. E.g.,
I can take this:
import javax.crypto.interfaces.DHPublicKey;
public class DhkeyTest {
public static void main(DHPublicKey key) {
System.err.println(key.getParams());
}
}
and compile using JDK 8 without any compile-time errors. So, it would make more
sense to me to not add the `@since` for it.
I believe Nizar will separate the changes to the Key interfaces into a separate
PR, so we can discuss in more detail there.
Thanks!
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18055#discussion_r1528625403
Re: RFR: JDK-8326951 Missing @since Tags [v2]
> I added `@since` tags for methods/constructors that do not match the `@since` > of the enclosing class. > > The `write` method already existed in `PrintStream` in earlier versions and > instances of it could always call this method, since it extends > `FilterOutputStream` [which has the > method](https://github.com/openjdk/jdk6/blob/3e49aa876353eaa215cde71eb21acc9b7f9872a0/jdk/src/share/classes/java/io/FilterOutputStream.java#L96). Nizar Benalla has updated the pull request incrementally with three additional commits since the last revision: - Revert "fix rest of private/public since tags" This reverts commit 2c04a9d8e773616b7b6239335d4e5eb955944ad1. - Revert "removed unnecessary @ since tags" This reverts commit 5da3f0d83d19393eeb3a9da68aac40dd999de660. - removed unnecessary @ since tags - Changes: - all: https://git.openjdk.org/jdk/pull/18055/files - new: https://git.openjdk.org/jdk/pull/18055/files/2c04a9d8..ba97724d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18055=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18055=00-01 Stats: 10 lines in 10 files changed: 8 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18055.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18055/head:pull/18055 PR: https://git.openjdk.org/jdk/pull/18055
Re: RFR: JDK-8326951 Missing @since Tags [v2]
On Sat, 16 Mar 2024 00:20:51 GMT, Jaikiran Pai wrote:
>> Nizar Benalla has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Revert "fix rest of private/public since tags"
>>
>>This reverts commit 2c04a9d8e773616b7b6239335d4e5eb955944ad1.
>> - Revert "removed unnecessary @ since tags"
>>
>>This reverts commit 5da3f0d83d19393eeb3a9da68aac40dd999de660.
>> - removed unnecessary @ since tags
>
> src/java.base/share/classes/javax/crypto/interfaces/DHPublicKey.java line 68:
>
>> 66: *
>> 67: * @return {@inheritDoc java.security.AsymmetricKey}
>> 68: * @since 22
>
> Hello Nizar, are the removal of `@since` in this PR intentional? I haven't
> checked all of them, but this specific removal appears incorrect, since this
> method was indeed introduced in Java 22 as part of
> https://bugs.openjdk.org/browse/JDK-8318096.
Hello Jaikiran,
in jdk21 DHPPublicKey did have a
[getParams()](https://github.com/openjdk/jdk21/blob/890adb6410dab4606a4f26a942aed02fb2f55387/src/java.base/share/classes/com/sun/crypto/provider/DHPublicKey.java#L244)
method, so it is not new in jdk 22. It also existed [before
that](https://github.com/openjdk/jdk11/blob/37115c8ea4aff13a8148ee2b8832b20888a5d880/src/java.base/share/classes/com/sun/crypto/provider/DHPublicKey.java#L252)
If you haven't looked at the other cases, I was about to group the changes
related to the Key interfaces in a separate PR if that's fine. let me know what
you think
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18055#discussion_r1527393223
