On Fri, 29 Nov 2024 11:38:58 GMT, Alan Bateman <[email protected]> wrote:
>> Volkan Yazıcı has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Use more fine-grained suppression
>> - Further simplify tests
>> - Extend `translateToSocketException()` with `AlreadyConnectedException`
>
> src/java.base/share/classes/sun/nio/ch/Net.java line 171:
>
>> 169: nx = newSocketException("Already bound");
>> 170: else if (x instanceof AlreadyConnectedException)
>> 171: nx = newSocketException("already connected");
>
> Probably better to use "Already connected" here, that will mean it has the
> same case as the exception message in SocketAdaptor.connect.
I deviated from the first-letter-in-upper-case convention deliberately to match
the message in `Socket`. Are you sure you want me to replace `already
connected` with `Already connected` in `Net`?
> test/jdk/java/net/Socket/ConnectFailTest.java line 72:
>
>> 70:
>> 71: /**
>> 72: * Verifies socket is closed when {@code unboundSocket.connect()}
>> fails.
>
> Did you mean to say "unboundSocket.connect"? I assume to mean to that it
> tests that an unbound Socket is closed when connect fails.
The current Javadoc is as follows:
Verifies socket is closed when {@code unboundSocket.connect()} fails.
Doesn't this express what you want me to say? How shall I rephrase it?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1863527844
PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1863525183
