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

Autocorrect BracesAroundHashParameters cop. #612

Merged

Conversation

dblock
Copy link
Contributor

@dblock dblock commented Nov 7, 2013

Pretty straightforward. I think the autocorrect could also get rid of the extra spacing, but I couldn't get that to work.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) when pulling c7d77b847121c7f215b49f2ceb997b1a6dc2bc50 on dblock:autocorrect-braces-around-hash-parameters into 415e648 on bbatsov:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) when pulling c7d77b847121c7f215b49f2ceb997b1a6dc2bc50 on dblock:autocorrect-braces-around-hash-parameters into 415e648 on bbatsov:master.

@@ -75,43 +75,48 @@

describe 'registers an offence for' do
it 'one non-hash parameter followed by a hash parameter with braces' do
inspect_source(cop, ['where(1, { y: 2 })'])
corrected = autocorrect_source(cop, ['where(1, { y: 2 })'])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally we keep the auto-correct examples separated from the detection examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, but I think it might be a bad idea. This duplicates the entire test for no added benefit, there's at least a dozen of those. Lmk if you still feel otherwise. Also note that autocorrect_source is inspect_source + autocorrect.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally have only a few auto-correct examples, since it's pretty obvious that if the detection examples work the auto-correction examples will also work. It's a matter of good style for spec examples to test just one thing at a time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, +10 on all this nitpicking. I am usually that guy doing the nitpicking and it's nice to find people who are even crazier than me about spaces and parenthesis.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) when pulling c1835205053dbfb015ea6cc37b7b0778dbafa096 on dblock:autocorrect-braces-around-hash-parameters into e4c8a22 on bbatsov:master.

@dblock
Copy link
Contributor Author

dblock commented Nov 7, 2013

Split autocorrect tests, this should be good to merge.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) when pulling 30fcdb9 on dblock:autocorrect-braces-around-hash-parameters into e4c8a22 on bbatsov:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) when pulling 30fcdb9 on dblock:autocorrect-braces-around-hash-parameters into e4c8a22 on bbatsov:master.

bbatsov added a commit that referenced this pull request Nov 7, 2013
…rameters

Autocorrect BracesAroundHashParameters cop.
@bbatsov bbatsov merged commit b62e186 into rubocop:master Nov 7, 2013
@dblock dblock deleted the autocorrect-braces-around-hash-parameters branch November 7, 2013 14:48
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.

3 participants