-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
do not catch checked Exception if is not thrown #2615
do not catch checked Exception if is not thrown #2615
Conversation
@bokshitsky any reason for closing this? ps: tests are now passing. |
Ouch. Had problems with local tests run - sentinel tests were falling - I though what my branch was not the reason. Thanks, reopen the PR |
@sazzad16 what do you think about these changes? |
@bokshitsky My comment about these changes in #2612 still stands. (PS: I get your concern as well.) So I'm just waiting for other reviewers to participate. |
* do not catch checked Exception if is not thrown * format import Co-authored-by: Guy Korland <gkorland@gmail.com> Co-authored-by: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com>
non significant update - tried to put it in previous pr #2612 but you suggested to put in separate
catching checked Exception instead of RuntimeException should be a avoided: it is dangerous due to code inside
try
block may add new checked exception with special handling required, but author can miss it due to no compile error (especially dangerous for InterruptedException)commit modifies all production files with this "code smell", do not touch tests files due to less dangerous consequences of possible error in them.