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

Fix bugs in IfUnlessModifier #1859

Merged
merged 3 commits into from
May 4, 2015

Conversation

jonas054
Copy link
Collaborator

@jonas054 jonas054 commented May 3, 2015

It's not entirely clear how best to handle comments around if statements when changing them to modifier form. I came to the conclusion that EOL comments right after the condition can be transferred by auto-correct, but comments after end need to be looked at by a person. See if you agree.

@bbatsov
Copy link
Collaborator

bbatsov commented May 4, 2015

Have a look at the failing build, please.

@jonas054 jonas054 force-pushed the fix_if_unless_modifier_bugs branch from bdbcf9c to 1a853c3 Compare May 4, 2015 14:31
@jonas054
Copy link
Collaborator Author

jonas054 commented May 4, 2015

Thanks for the reminder! I took the easy way out this time and just upped the MethodLength: Max. I will do a refactoring PR later that brings it back down.

@bbatsov
Copy link
Collaborator

bbatsov commented May 4, 2015

You'll also have to rebase this.

An EOL comment that follows the condition should
still be around after auto-correcting to modifier
form.
Just like comments in the body, an EOL comment after end may
be difficult to put in a good place, if the statement is
changed to modifier form.
Only code and comment lines should count when checking
a multiline if.
@jonas054 jonas054 force-pushed the fix_if_unless_modifier_bugs branch from 1a853c3 to 9f771a4 Compare May 4, 2015 16:16
@jonas054
Copy link
Collaborator Author

jonas054 commented May 4, 2015

Rebased.

bbatsov added a commit that referenced this pull request May 4, 2015
@bbatsov bbatsov merged commit 4b7b0cd into rubocop:master May 4, 2015
@bbatsov
Copy link
Collaborator

bbatsov commented May 4, 2015

👍

@jonas054 jonas054 deleted the fix_if_unless_modifier_bugs branch May 5, 2015 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants