Update spotbugs 4.6.0 -> 4.7.0 #2222
Conversation
Scottmitch
changed the title
| @@ -157,7 +158,7 @@ private static void shutdownExecutor(java.util.concurrent.Executor jdkExecutor) | |||
| try { | |||
| ((AutoCloseable) jdkExecutor).close(); | |||
| } catch (Exception e) { | |||
| throw new RuntimeException("unexpected exception while closing executor: " + jdkExecutor, e); | |||
| throwException(e); | |||
There was a problem hiding this comment.
most cases I added an exception to ignore the new spotbugs warnings, but some cases were either expected to be rare or otherwise seemed to make sense to re-throw or choose to use some other unchecked exception
There was a problem hiding this comment.
This will potentially change the type of the exception thrown. Wrapping the exception as a RuntimeException is gross, but it is at least consistent.
There was a problem hiding this comment.
both ways are not ideal, don't have any strong preference
There was a problem hiding this comment.
I will revert the change for this PR and submit in a followup for discussion and to make it more clear in release notes if we pull it in.
Scottmitch
force-pushed
the
find_bugs
branch
3 times, most recently
from
9c0ccc8 to
62f9095
Compare
| @@ -148,7 +148,7 @@ public void onError(final Throwable t) { | |||
|
|
|||
| private Subscriber checkSubscriberAndExceptions() { | |||
| if (!exceptions.isEmpty()) { | |||
| final RuntimeException exception = new RuntimeException("Unexpected exception(s) encountered", | |||
| final IllegalStateException exception = new IllegalStateException("Unexpected exception(s) encountered", | |||
There was a problem hiding this comment.
If it's a test utility, should we use AssertionError in this case for TestCompletable, TestSingle, and TestPublisher?
| @@ -157,7 +158,7 @@ private static void shutdownExecutor(java.util.concurrent.Executor jdkExecutor) | |||
| try { | |||
| ((AutoCloseable) jdkExecutor).close(); | |||
| } catch (Exception e) { | |||
| throw new RuntimeException("unexpected exception while closing executor: " + jdkExecutor, e); | |||
| throwException(e); | |||
There was a problem hiding this comment.
both ways are not ideal, don't have any strong preference
No description provided.