-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adds some conditional examples for Ruby #6
base: master
Are you sure you want to change the base?
Conversation
@trevororeilly, can you take a look at this update, see if it looks good based on what we talked about? |
end | ||
|
||
# good | ||
do_something if some_condition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if do_something or some_conditions is too long I would avoid modifiers. For good example I would have something specific like:
a = 1 if b.present?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I tried to capture the long-case in the first #bad example. Do you think think needs a better example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you mean the "do_something" and "some_condition"... I'd like to keep those as general as possible. I'm modeling this change on someone else's style-guide: https://github.com/bbatsov/ruby-style-guide#if-as-a-modifier which is very easy to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could maybe start by specifically allowing certain cases like the single-line return:
- Favor modifier
if/unless
usage when you have a simple single-line return statement. - Avoid modifier
if/unless
usage at the end of a non-trivial statement or multi-line block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is covered in the current PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, was just thinking that you could avoid uncertainty around which situations are allowed if the rule is really specific.
No description provided.