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

Disable Style/RaiseArgs #38

Merged
merged 1 commit into from Dec 5, 2018

Conversation

Projects
None yet
2 participants
@knu
Copy link
Contributor

knu commented Dec 5, 2018

The recommended style raise RuntimeError, "message" generally looks better than raise RuntimeError.new("message") because it is a variant of raise "message" with an explicit exception class supplied. However, when the argument is not a message string, the comma style does not read well because the second argument is not what raise would take but is just an argument for the first argument's constructor that's passed through.

e.g.

raise ActiveRecord::RecordInvalid, record

So, I'm not in favor of forcing this style unless this cop learns to do the conversion only if the second argument is a string literal.

What do you think?

Disable Style/RaiseArgs
The recommended style `raise RuntimeError, "message"` generally looks
better than `raise RuntimeError.new("message")` because it is a
variant of `raise "message"` with an explicit exception class
supplied.  However, when the argument is not a message string, the
comma style does not read well because the second argument is not what
`raise` would take but is just an argument for the first argument's
constructor that's passed through.

e.g.
```ruby
raise ActiveRecord::RecordInvalid, record
```

So, I'm not in favor of forcing this style unless this cop learns to
do the conversion only if the second argument is a string literal.

@knu knu force-pushed the knu:disable-RaiseArgs branch from ad9ee15 to 37e4468 Dec 5, 2018

@searls

This comment has been minimized.

Copy link
Member

searls commented Dec 5, 2018

Agree. This seems like a fight nobody was asking for.

@searls searls closed this Dec 5, 2018

@searls searls reopened this Dec 5, 2018

@searls searls merged commit f9746b3 into testdouble:master Dec 5, 2018

@knu knu deleted the knu:disable-RaiseArgs branch Dec 7, 2018

@knu

This comment has been minimized.

Copy link
Contributor Author

knu commented Dec 7, 2018

Thanks for merging! Now let me ask you something. I simply removed these lines because the file only contained Enabled: true settings, but could it be a good idea if we kept this setting with Enabled: false or a commented out section to make it clear that we explicitly opted it out for a reason?

@searls

This comment has been minimized.

Copy link
Member

searls commented Dec 8, 2018

Ideally I want the config to be as brief as possible (while still being disable-by-default and complete, so that changes to the Rubocop default ruleset do not inadvertently change Standard). If someone has a question about a specific rule, I hope they will search GitHub or git log -S "Style/RaiseArgs" to learn what happened previously

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.