You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "orionlibs (via GitHub)" <gi...@apache.org> on 2023/06/29 20:23:48 UTC

[GitHub] [commons-lang] orionlibs opened a new pull request, #1069: LANG-1647

orionlibs opened a new pull request, #1069:
URL: https://github.com/apache/commons-lang/pull/1069

   --created method in ExceptionUtils that checks if a given Throwable represents a checked exception.
   --created method in ExceptionUtils that checks if a given Throwable represents an unchecked exception.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] orionlibs commented on a diff in pull request #1069: LANG-1647

Posted by "orionlibs (via GitHub)" <gi...@apache.org>.
orionlibs commented on code in PR #1069:
URL: https://github.com/apache/commons-lang/pull/1069#discussion_r1249605535


##########
src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java:
##########
@@ -850,4 +850,40 @@ public void testWrapAndUnwrapThrowable() {
         final Throwable t = assertThrows(Throwable.class, () -> ExceptionUtils.wrapAndThrow(new TestThrowable()));
         assertTrue(ExceptionUtils.hasCause(t, TestThrowable.class));
     }
+
+    @Test
+    public void testIsChecked_unchecked() {
+        final boolean result = ExceptionUtils.isChecked(new IllegalArgumentException());

Review Comment:
   @garydgregory  so, I don't need to create local variables if the assertion looks like this:
   assertTrue(ExceptionUtils.isChecked(new IllegalArgumentException()));
   Is that what you mean? Is that what I need to do for this PR?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] orionlibs commented on a diff in pull request #1069: LANG-1647

Posted by "orionlibs (via GitHub)" <gi...@apache.org>.
orionlibs commented on code in PR #1069:
URL: https://github.com/apache/commons-lang/pull/1069#discussion_r1249563312


##########
src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java:
##########
@@ -850,4 +850,40 @@ public void testWrapAndUnwrapThrowable() {
         final Throwable t = assertThrows(Throwable.class, () -> ExceptionUtils.wrapAndThrow(new TestThrowable()));
         assertTrue(ExceptionUtils.hasCause(t, TestThrowable.class));
     }
+
+    @Test
+    public void testIsChecked_unchecked() {
+        final boolean result = ExceptionUtils.isChecked(new IllegalArgumentException());

Review Comment:
   @garydgregory so, should I create new local variables for each case?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on pull request #1069: LANG-1647

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #1069:
URL: https://github.com/apache/commons-lang/pull/1069#issuecomment-1616686083

   -1 as is, in https://issues.apache.org/jira/browse/LANG-1647 the description reads:
   ```
   The idea it's have a function that check if a given throwable is a checked exception. There are some similar function in ConcurrentUtils, but have package visibility and return the same exception. 
   
   Seem logic have this verification in the class that have this responsibility - ExceptionUtils
   ```
   `ConcurrentUtils` and anywhere else in Commons Lang should reuse these new APIs, otherwise, the PR just creates duplication of existing code patterns.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on a diff in pull request #1069: LANG-1647

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #1069:
URL: https://github.com/apache/commons-lang/pull/1069#discussion_r1249606014


##########
src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java:
##########
@@ -850,4 +850,40 @@ public void testWrapAndUnwrapThrowable() {
         final Throwable t = assertThrows(Throwable.class, () -> ExceptionUtils.wrapAndThrow(new TestThrowable()));
         assertTrue(ExceptionUtils.hasCause(t, TestThrowable.class));
     }
+
+    @Test
+    public void testIsChecked_unchecked() {
+        final boolean result = ExceptionUtils.isChecked(new IllegalArgumentException());

Review Comment:
   πŸ‘πŸ˜‰



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] orionlibs commented on a diff in pull request #1069: LANG-1647

Posted by "orionlibs (via GitHub)" <gi...@apache.org>.
orionlibs commented on code in PR #1069:
URL: https://github.com/apache/commons-lang/pull/1069#discussion_r1249605535


##########
src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java:
##########
@@ -850,4 +850,40 @@ public void testWrapAndUnwrapThrowable() {
         final Throwable t = assertThrows(Throwable.class, () -> ExceptionUtils.wrapAndThrow(new TestThrowable()));
         assertTrue(ExceptionUtils.hasCause(t, TestThrowable.class));
     }
+
+    @Test
+    public void testIsChecked_unchecked() {
+        final boolean result = ExceptionUtils.isChecked(new IllegalArgumentException());

Review Comment:
   @garydgregory  so, I don't need to create local variables if the assertion looks like this:
   assertTrue(ExceptionUtils.isChecked(new IllegalArgumentException()));
   Is that what you mean?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on a diff in pull request #1069: LANG-1647

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #1069:
URL: https://github.com/apache/commons-lang/pull/1069#discussion_r1249604113


##########
src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java:
##########
@@ -850,4 +850,40 @@ public void testWrapAndUnwrapThrowable() {
         final Throwable t = assertThrows(Throwable.class, () -> ExceptionUtils.wrapAndThrow(new TestThrowable()));
         assertTrue(ExceptionUtils.hasCause(t, TestThrowable.class));
     }
+
+    @Test
+    public void testIsChecked_unchecked() {
+        final boolean result = ExceptionUtils.isChecked(new IllegalArgumentException());

Review Comment:
   > @garydgregory so, should I create new local variables for each case?
   
   You've got it backward.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on a diff in pull request #1069: LANG-1647

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #1069:
URL: https://github.com/apache/commons-lang/pull/1069#discussion_r1249556051


##########
src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java:
##########
@@ -850,4 +850,40 @@ public void testWrapAndUnwrapThrowable() {
         final Throwable t = assertThrows(Throwable.class, () -> ExceptionUtils.wrapAndThrow(new TestThrowable()));
         assertTrue(ExceptionUtils.hasCause(t, TestThrowable.class));
     }
+
+    @Test
+    public void testIsChecked_unchecked() {
+        final boolean result = ExceptionUtils.isChecked(new IllegalArgumentException());

Review Comment:
   @orionlibs 
   There is no need for a single-use local variable here and in other new tests.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on a diff in pull request #1069: LANG-1647

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #1069:
URL: https://github.com/apache/commons-lang/pull/1069#discussion_r1249622583


##########
src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java:
##########
@@ -850,4 +850,40 @@ public void testWrapAndUnwrapThrowable() {
         final Throwable t = assertThrows(Throwable.class, () -> ExceptionUtils.wrapAndThrow(new TestThrowable()));
         assertTrue(ExceptionUtils.hasCause(t, TestThrowable.class));
     }
+
+    @Test
+    public void testIsChecked_unchecked() {
+        final boolean result = ExceptionUtils.isChecked(new IllegalArgumentException());

Review Comment:
   Not quite, see https://github.com/apache/commons-lang/pull/1069#issuecomment-1616686083



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] codecov-commenter commented on pull request #1069: LANG-1647

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1069:
URL: https://github.com/apache/commons-lang/pull/1069#issuecomment-1613787385

   ## [Codecov](https://app.codecov.io/gh/apache/commons-lang/pull/1069?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   > Merging [#1069](https://app.codecov.io/gh/apache/commons-lang/pull/1069?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (0892777) into [master](https://app.codecov.io/gh/apache/commons-lang/commit/f4665d0bc604ed9bb1aae3c93e99d4eee5027c1a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (f4665d0) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   ```diff
   @@            Coverage Diff            @@
   ##             master    #1069   +/-   ##
   =========================================
     Coverage     92.07%   92.08%           
   - Complexity     7501     7506    +5     
   =========================================
     Files           194      194           
     Lines         15743    15745    +2     
     Branches       2902     2902           
   =========================================
   + Hits          14496    14498    +2     
     Misses          674      674           
     Partials        573      573           
   ```
   
   
   | [Impacted Files](https://app.codecov.io/gh/apache/commons-lang/pull/1069?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ” | |
   |---|---|---|
   | [...apache/commons/lang3/exception/ExceptionUtils.java](https://app.codecov.io/gh/apache/commons-lang/pull/1069?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbGFuZzMvZXhjZXB0aW9uL0V4Y2VwdGlvblV0aWxzLmphdmE=) | `97.56% <100.00%> (+0.03%)` | :arrow_up: |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory merged pull request #1069: LANG-1647

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #1069:
URL: https://github.com/apache/commons-lang/pull/1069


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] orionlibs commented on a diff in pull request #1069: LANG-1647

Posted by "orionlibs (via GitHub)" <gi...@apache.org>.
orionlibs commented on code in PR #1069:
URL: https://github.com/apache/commons-lang/pull/1069#discussion_r1249608304


##########
src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java:
##########
@@ -850,4 +850,40 @@ public void testWrapAndUnwrapThrowable() {
         final Throwable t = assertThrows(Throwable.class, () -> ExceptionUtils.wrapAndThrow(new TestThrowable()));
         assertTrue(ExceptionUtils.hasCause(t, TestThrowable.class));
     }
+
+    @Test
+    public void testIsChecked_unchecked() {
+        final boolean result = ExceptionUtils.isChecked(new IllegalArgumentException());

Review Comment:
   done!



##########
src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java:
##########
@@ -850,4 +850,40 @@ public void testWrapAndUnwrapThrowable() {
         final Throwable t = assertThrows(Throwable.class, () -> ExceptionUtils.wrapAndThrow(new TestThrowable()));
         assertTrue(ExceptionUtils.hasCause(t, TestThrowable.class));
     }
+
+    @Test
+    public void testIsChecked_unchecked() {
+        final boolean result = ExceptionUtils.isChecked(new IllegalArgumentException());

Review Comment:
   done! I updated the PR



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] orionlibs commented on a diff in pull request #1069: LANG-1647

Posted by "orionlibs (via GitHub)" <gi...@apache.org>.
orionlibs commented on code in PR #1069:
URL: https://github.com/apache/commons-lang/pull/1069#discussion_r1249641820


##########
src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java:
##########
@@ -850,4 +850,40 @@ public void testWrapAndUnwrapThrowable() {
         final Throwable t = assertThrows(Throwable.class, () -> ExceptionUtils.wrapAndThrow(new TestThrowable()));
         assertTrue(ExceptionUtils.hasCause(t, TestThrowable.class));
     }
+
+    @Test
+    public void testIsChecked_unchecked() {
+        final boolean result = ExceptionUtils.isChecked(new IllegalArgumentException());

Review Comment:
   @garydgregory you are right. I made ConcurrentUtils to use the new method. I hope this completes this PR. Thank you for pointing that comment out. I focused on other parts and I missed that part of the comment. Distractions are bad



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] orionlibs commented on a diff in pull request #1069: LANG-1647

Posted by "orionlibs (via GitHub)" <gi...@apache.org>.
orionlibs commented on code in PR #1069:
URL: https://github.com/apache/commons-lang/pull/1069#discussion_r1249567341


##########
src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java:
##########
@@ -850,4 +850,40 @@ public void testWrapAndUnwrapThrowable() {
         final Throwable t = assertThrows(Throwable.class, () -> ExceptionUtils.wrapAndThrow(new TestThrowable()));
         assertTrue(ExceptionUtils.hasCause(t, TestThrowable.class));
     }
+
+    @Test
+    public void testIsChecked_unchecked() {
+        final boolean result = ExceptionUtils.isChecked(new IllegalArgumentException());

Review Comment:
   @garydgregory what should I do for this PR to be successful?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org