Re: RFR: JDK-8282686: Add constructors taking a cause to SocketException [v3]
On Mon, 7 Mar 2022 17:55:45 GMT, Joe Darcy wrote:
>> Please review this small API enhancement to add the usual constructors
>> taking a cause to SocketException and then update uses of initiCause on
>> creating SocketException to instead pass the cause via the constructor.
>>
>> Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688
>
> Joe Darcy has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Improve test.
Hmm, this seems to have been integrated without any Reviewer on the final
commit. How did that happen?
test/jdk/java/net/SocketException/TestSocketExceptionCtor.java line 32:
> 30: import java.util.Objects;
> 31:
> 32: public class TestSocketExceptionCtor {
I don't think this is a good name for the test because it tests mores than the
constructors. So I think drop the suffix from the name.
-
PR: https://git.openjdk.java.net/jdk/pull/7705
Re: RFR: JDK-8282686: Add constructors taking a cause to SocketException [v3]
> Please review this small API enhancement to add the usual constructors taking > a cause to SocketException and then update uses of initiCause on creating > SocketException to instead pass the cause via the constructor. > > Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688 Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Improve test. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7705/files - new: https://git.openjdk.java.net/jdk/pull/7705/files/978b379d..da863692 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7705=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7705=01-02 Stats: 11 lines in 1 file changed: 8 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/7705.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7705/head:pull/7705 PR: https://git.openjdk.java.net/jdk/pull/7705
Re: RFR: JDK-8282686: Add constructors taking a cause to SocketException [v2]
> Please review this small API enhancement to add the usual constructors taking > a cause to SocketException and then update uses of initiCause on creating > SocketException to instead pass the cause via the constructor. > > Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688 Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Add regression test. - Merge branch 'master' into JDK-8282686 - JDK-8282686: Add constructors take a cause to SocketException - Changes: - all: https://git.openjdk.java.net/jdk/pull/7705/files - new: https://git.openjdk.java.net/jdk/pull/7705/files/e65f9ae5..978b379d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7705=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7705=00-01 Stats: 8268 lines in 175 files changed: 6072 ins; 1565 del; 631 mod Patch: https://git.openjdk.java.net/jdk/pull/7705.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7705/head:pull/7705 PR: https://git.openjdk.java.net/jdk/pull/7705
Re: RFR: JDK-8282686: Add constructors taking a cause to SocketException
On Fri, 4 Mar 2022 21:17:01 GMT, Joe Darcy wrote: > Please review this small API enhancement to add the usual constructors taking > a cause to SocketException and then update uses of initiCause on creating > SocketException to instead pass the cause via the constructor. > > Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688 Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7705
Re: RFR: JDK-8282686: Add constructors taking a cause to SocketException
On Fri, 4 Mar 2022 21:17:01 GMT, Joe Darcy wrote: > Please review this small API enhancement to add the usual constructors taking > a cause to SocketException and then update uses of initiCause on creating > SocketException to instead pass the cause via the constructor. > > Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688 Looks fine, would be worth including a couple of tests for coverage - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7705
Re: RFR: JDK-8282686: Add constructors taking a cause to SocketException
On Fri, 4 Mar 2022 21:17:01 GMT, Joe Darcy wrote: > Please review this small API enhancement to add the usual constructors taking > a cause to SocketException and then update uses of initiCause on creating > SocketException to instead pass the cause via the constructor. > > Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688 Looks good to me. I may file a new RFE and use this constructor in the JSSE implementation. - Marked as reviewed by xuelei (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7705
Re: RFR: JDK-8282686: Add constructors taking a cause to SocketException
On Fri, 4 Mar 2022 21:17:01 GMT, Joe Darcy wrote: > Please review this small API enhancement to add the usual constructors taking > a cause to SocketException and then update uses of initiCause on creating > SocketException to instead pass the cause via the constructor. > > Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688 This looks good, I just wonder if we should add a test for the new constructors. One of them will be tested by the usages in the JDK, the cause-only constructor may not be exercised by any code. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7705
