Re: RFR: 8332181: Deprecate for removal the java.net.MulticastSocket.setTTL/getTTL and the 2-arg send methods [v2]
On Fri, 17 May 2024 11:50:15 GMT, Jaikiran Pai wrote:
>> Can I please get a review of this change which proposes to deprecate for
>> removal 3 methods on `java.net.MulticastSocket`? This addresses
>> https://bugs.openjdk.org/browse/JDK-8332181.
>>
>> As noted in that issue these methods have been deprecated since Java 1.2 and
>> 1.4 days. They currently have replacement methods (noted in their javadoc)
>> which have been in use for several releases. This commit updates these
>> deprecated methods to deprecated for removal, to allow for their removal in
>> a future release.
>>
>> No new tests have been added and existing tests in tier1, tier2 and tier3
>> continue to pass.
>
> Jaikiran Pai has updated the pull request incrementally with one additional
> commit since the last revision:
>
> also add forRemoval in internal classes
I think DatagramSocketImpl.getTTL/setTTL will need to be deprecated for removal
at the same time.
Also I assume @Deprecated can be dropped from DatagramSocketAdaptor and
NetMulticastSocket, they just override or call the deprecated methods so should
only need `@SuppressWarnings("removal")`.
-
PR Comment: https://git.openjdk.org/jdk/pull/19242#issuecomment-2119261576
Re: RFR: 8332181: Deprecate for removal the java.net.MulticastSocket.setTTL/getTTL and the 2-arg send methods [v2]
On Fri, 17 May 2024 11:50:15 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which proposes to deprecate for >> removal 3 methods on `java.net.MulticastSocket`? This addresses >> https://bugs.openjdk.org/browse/JDK-8332181. >> >> As noted in that issue these methods have been deprecated since Java 1.2 and >> 1.4 days. They currently have replacement methods (noted in their javadoc) >> which have been in use for several releases. This commit updates these >> deprecated methods to deprecated for removal, to allow for their removal in >> a future release. >> >> No new tests have been added and existing tests in tier1, tier2 and tier3 >> continue to pass. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > also add forRemoval in internal classes Thank you Iris and Daniel for the reviews. I've moved the CSR to Finalized. - PR Comment: https://git.openjdk.org/jdk/pull/19242#issuecomment-2119092723
Re: RFR: 8332181: Deprecate for removal the java.net.MulticastSocket.setTTL/getTTL and the 2-arg send methods [v2]
On Fri, 17 May 2024 11:50:15 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which proposes to deprecate for >> removal 3 methods on `java.net.MulticastSocket`? This addresses >> https://bugs.openjdk.org/browse/JDK-8332181. >> >> As noted in that issue these methods have been deprecated since Java 1.2 and >> 1.4 days. They currently have replacement methods (noted in their javadoc) >> which have been in use for several releases. This commit updates these >> deprecated methods to deprecated for removal, to allow for their removal in >> a future release. >> >> No new tests have been added and existing tests in tier1, tier2 and tier3 >> continue to pass. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > also add forRemoval in internal classes >From the JCP POV, these changes look fine. These changes match the associated >CSR, which I've also Reviewed. - Marked as reviewed by iris (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19242#pullrequestreview-2064034102
Re: RFR: 8332181: Deprecate for removal the java.net.MulticastSocket.setTTL/getTTL and the 2-arg send methods [v2]
On Fri, 17 May 2024 11:50:15 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which proposes to deprecate for >> removal 3 methods on `java.net.MulticastSocket`? This addresses >> https://bugs.openjdk.org/browse/JDK-8332181. >> >> As noted in that issue these methods have been deprecated since Java 1.2 and >> 1.4 days. They currently have replacement methods (noted in their javadoc) >> which have been in use for several releases. This commit updates these >> deprecated methods to deprecated for removal, to allow for their removal in >> a future release. >> >> No new tests have been added and existing tests in tier1, tier2 and tier3 >> continue to pass. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > also add forRemoval in internal classes Marked as reviewed by dfuchs (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19242#pullrequestreview-2063259181
Re: RFR: 8332181: Deprecate for removal the java.net.MulticastSocket.setTTL/getTTL and the 2-arg send methods [v2]
On Fri, 17 May 2024 11:47:34 GMT, Jaikiran Pai wrote:
>> src/java.base/share/classes/java/net/NetMulticastSocket.java line 655:
>>
>>> 653: @Deprecated
>>> 654: @Override
>>> 655: @SuppressWarnings("removal")
>>
>> Instead of adding `@SuppressWarning("removal")` here (and at other places
>> below) - would it make sense to update the `@Deprecated` annotation and add
>> `forRemoval = true`?
>
> We will still have to add the `@SuppressWarning("removal")` to these methods
> otherwise the compilation generates a warning which fails the build:
>
>
> java.base/share/classes/java/net/NetMulticastSocket.java:655: warning:
> [removal] setTTL(byte) in MulticastSocket has been deprecated and marked for
> removal
> public void setTTL(byte ttl) throws IOException {
> ^
> java.base/share/classes/java/net/NetMulticastSocket.java:673: warning:
> [removal] getTTL() in MulticastSocket has been deprecated and marked for
> removal
> public byte getTTL() throws IOException {
> ^
> error: warnings found and -Werror specified
> 1 error
> 2 warnings
>
>
> I went ahead and updated these internal `NetMulticastSocket` and
> `DatagramSocketAdaptor` classes to add the `forRemoval` attribute to the
> `@Deprecated` annotation to keep it consistent.
Ah - thank you. I had hoped that adding forRemoval = true to the annotation
would allow us to get rid of the SuppressWarning.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19242#discussion_r1604901662
Re: RFR: 8332181: Deprecate for removal the java.net.MulticastSocket.setTTL/getTTL and the 2-arg send methods [v2]
> Can I please get a review of this change which proposes to deprecate for > removal 3 methods on `java.net.MulticastSocket`? This addresses > https://bugs.openjdk.org/browse/JDK-8332181. > > As noted in that issue these methods have been deprecated since Java 1.2 and > 1.4 days. They currently have replacement methods (noted in their javadoc) > which have been in use for several releases. This commit updates these > deprecated methods to deprecated for removal, to allow for their removal in a > future release. > > No new tests have been added and existing tests in tier1, tier2 and tier3 > continue to pass. Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: also add forRemoval in internal classes - Changes: - all: https://git.openjdk.org/jdk/pull/19242/files - new: https://git.openjdk.org/jdk/pull/19242/files/b34fbe08..6cece088 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19242=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19242=00-01 Stats: 6 lines in 2 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/19242.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19242/head:pull/19242 PR: https://git.openjdk.org/jdk/pull/19242
Re: RFR: 8332181: Deprecate for removal the java.net.MulticastSocket.setTTL/getTTL and the 2-arg send methods [v2]
On Fri, 17 May 2024 11:09:31 GMT, Daniel Fuchs wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> also add forRemoval in internal classes
>
> src/java.base/share/classes/java/net/NetMulticastSocket.java line 655:
>
>> 653: @Deprecated
>> 654: @Override
>> 655: @SuppressWarnings("removal")
>
> Instead of adding `@SuppressWarning("removal")` here (and at other places
> below) - would it make sense to update the `@Deprecated` annotation and add
> `forRemoval = true`?
We will still have to add the `@SuppressWarning("removal")` to these methods
otherwise the compilation generates a warning which fails the build:
java.base/share/classes/java/net/NetMulticastSocket.java:655: warning:
[removal] setTTL(byte) in MulticastSocket has been deprecated and marked for
removal
public void setTTL(byte ttl) throws IOException {
^
java.base/share/classes/java/net/NetMulticastSocket.java:673: warning:
[removal] getTTL() in MulticastSocket has been deprecated and marked for removal
public byte getTTL() throws IOException {
^
error: warnings found and -Werror specified
1 error
2 warnings
I went ahead and updated these internal `NetMulticastSocket` and
`DatagramSocketAdaptor` classes to add the `forRemoval` attribute to the
`@Deprecated` annotation to keep it consistent.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19242#discussion_r1604848467
