Re: RFR: 8284415: Collapse identical catch branches in security libs [v2]
On Thu, 7 Apr 2022 07:52:29 GMT, Andrey Turbanov wrote: >> Let's take advantage of Java 7 language feature - "Catching Multiple >> Exception Types". >> It simplifies code. Reduces duplication. >> Found by IntelliJ IDEA inspection `Identical 'catch' branches in 'try' >> statement` > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8284415: Collapse identical catch branches in security libs > update one more copyright Thank you for review! - PR: https://git.openjdk.java.net/jdk/pull/8068
Re: RFR: 8284415: Collapse identical catch branches in security libs [v2]
> Let's take advantage of Java 7 language feature - "Catching Multiple > Exception Types". > It simplifies code. Reduces duplication. > Found by IntelliJ IDEA inspection `Identical 'catch' branches in 'try' > statement` Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8284415: Collapse identical catch branches in security libs update one more copyright - Changes: - all: https://git.openjdk.java.net/jdk/pull/8068/files - new: https://git.openjdk.java.net/jdk/pull/8068/files/3dd769bc..1e7109d3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8068&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8068&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8068.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8068/head:pull/8068 PR: https://git.openjdk.java.net/jdk/pull/8068
Re: RFR: 8284415: Collapse identical catch branches in security libs
On Fri, 1 Apr 2022 07:32:21 GMT, Andrey Turbanov wrote:
> Let's take advantage of Java 7 language feature - "Catching Multiple
> Exception Types".
> It simplifies code. Reduces duplication.
> Found by IntelliJ IDEA inspection `Identical 'catch' branches in 'try'
> statement`
Other that the 1 copyright issue, LGTM.
I didn't notice any expect behavioral changes between the new and old code.
src/jdk.security.auth/share/classes/com/sun/security/auth/module/Krb5LoginModule.java
line 788:
> 786:
> 787: }
> 788: } catch (KrbException | IOException e) {
Please update copyright date.
-
Marked as reviewed by wetmore (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8068
Re: RFR: 8284415: Collapse identical catch branches in security libs
On Fri, 1 Apr 2022 07:32:21 GMT, Andrey Turbanov wrote: > Let's take advantage of Java 7 language feature - "Catching Multiple > Exception Types". > It simplifies code. Reduces duplication. > Found by IntelliJ IDEA inspection `Identical 'catch' branches in 'try' > statement` I think changing behavior (like adding cause) requires at least writing regression test. I would like to not do this under this PR. - PR: https://git.openjdk.java.net/jdk/pull/8068
Re: [Internet]Re: RFR: 8284415: Collapse identical catch branches in security libs
I think it is a good point, Mike. Some of the exception constructors do not accept cause parameters, for example UnrecoverableKeyException. And some others do, for example InvalidKeyException. I would like to keep original cause. I read this patch as a format clean-up. It is fine to me to have an additional update to take the causes back. But I appreciate if Andrey could take care of the update in the PR as well. Xuelei > On Apr 6, 2022, at 10:43 AM, Mike StJohns wrote: > > Before you approve this, why would you rethrow just the message rather than > the actual original cause? Seems like you’re losing debug info including the > type of the original exception for no good reason. > > Mike > > Sent from my iPad > >> On Apr 6, 2022, at 11:26, Xue-Lei Andrew Fan wrote: >> >> On Fri, 1 Apr 2022 07:32:21 GMT, Andrey Turbanov >> wrote: >> >>> Let's take advantage of Java 7 language feature - "Catching Multiple >>> Exception Types". >>> It simplifies code. Reduces duplication. >>> Found by IntelliJ IDEA inspection `Identical 'catch' branches in 'try' >>> statement` >> >> Looks good to me. >> >> - >> >> Marked as reviewed by xuelei (Reviewer). >> >> PR: https://git.openjdk.java.net/jdk/pull/8068 > >
Re: RFR: 8284415: Collapse identical catch branches in security libs
Before you approve this, why would you rethrow just the message rather than the actual original cause? Seems like you’re losing debug info including the type of the original exception for no good reason. Mike Sent from my iPad > On Apr 6, 2022, at 11:26, Xue-Lei Andrew Fan wrote: > > On Fri, 1 Apr 2022 07:32:21 GMT, Andrey Turbanov > wrote: > >> Let's take advantage of Java 7 language feature - "Catching Multiple >> Exception Types". >> It simplifies code. Reduces duplication. >> Found by IntelliJ IDEA inspection `Identical 'catch' branches in 'try' >> statement` > > Looks good to me. > > - > > Marked as reviewed by xuelei (Reviewer). > > PR: https://git.openjdk.java.net/jdk/pull/8068
Re: RFR: 8284415: Collapse identical catch branches in security libs
On Fri, 1 Apr 2022 07:32:21 GMT, Andrey Turbanov wrote: > Let's take advantage of Java 7 language feature - "Catching Multiple > Exception Types". > It simplifies code. Reduces duplication. > Found by IntelliJ IDEA inspection `Identical 'catch' branches in 'try' > statement` Looks good to me. - Marked as reviewed by xuelei (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8068
Re: RFR: 8284415: Collapse identical catch branches in security libs
On Fri, 1 Apr 2022 07:32:21 GMT, Andrey Turbanov wrote: > Let's take advantage of Java 7 language feature - "Catching Multiple > Exception Types". > It simplifies code. Reduces duplication. > Found by IntelliJ IDEA inspection `Identical 'catch' branches in 'try' > statement` Looks fine! - Marked as reviewed by coffeys (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8068
RFR: 8284415: Collapse identical catch branches in security libs
Let's take advantage of Java 7 language feature - "Catching Multiple Exception Types". It simplifies code. Reduces duplication. Found by IntelliJ IDEA inspection `Identical 'catch' branches in 'try' statement` - Commit messages: - [PATCH] Collapse identical catch branches in security libs - [PATCH] Collapse identical catch branches in security libs Changes: https://git.openjdk.java.net/jdk/pull/8068/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8068&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284415 Stats: 197 lines in 32 files changed: 1 ins; 124 del; 72 mod Patch: https://git.openjdk.java.net/jdk/pull/8068.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8068/head:pull/8068 PR: https://git.openjdk.java.net/jdk/pull/8068
