Skip to content
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

Set Style/RescueStandardError to implicit #310

Merged
merged 1 commit into from
Jan 13, 2020

Conversation

Fryguy
Copy link
Member

@Fryguy Fryguy commented May 29, 2018

Most of the code base is already in implicit mode, but rubocop changed
their default to explicit. This moves it back for consistency as well
as my personal preference :)

https://www.rubydoc.info/gems/rubocop/0.52.1/RuboCop/Cop/Style/RescueStandardError

Please vote on what you prefer with 👍 or 👎 on this comment (:+1: means you want implicit like this PR; :-1: means you want explicit as per the rubocop default)

Most of the code base is already in implicit mode, but rubocop changed
their default to explicit.  This moves it back for consistency as well
as my personal preference :)
@bdunne
Copy link
Member

bdunne commented May 29, 2018

I think I prefer explicit to encourage people to rescue only the exceptions they are expecting.

@Fryguy
Copy link
Member Author

Fryguy commented May 29, 2018

Strangely, the Ruby Style Guide itself, says that rescue => err is good: https://github.com/bbatsov/ruby-style-guide#no-blind-rescues

There doesn't seem to be an entry in the style guide for this particular cop.

@Fryguy
Copy link
Member Author

Fryguy commented May 29, 2018

Opened an issue on ruby-style-guide about the inconsistency - rubocop/ruby-style-guide#719

@djberg96
Copy link
Contributor

djberg96 commented Jun 6, 2019

I'm used to implicit, but don't have any objections to making it explicit. Just means lots of cleanup.

@bdunne bdunne merged commit d320f28 into ManageIQ:master Jan 13, 2020
@bdunne bdunne self-assigned this Jan 13, 2020
@Fryguy Fryguy deleted the style-rescue-standard-error branch January 15, 2020 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants