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 ambiguous blocks in specs. #19

Merged
merged 1 commit into from May 11, 2018
Merged

Disable ambiguous blocks in specs. #19

merged 1 commit into from May 11, 2018

Conversation

bwebster
Copy link
Contributor

Currently, we get rubocop linter issues for code like this

  it "does not clone manage access defaults" do
    expect { perform_action }.to_not change { Racl::Default.count }
  end

This is valid rspec syntax and should not trigger a linter error.

See rubocop/rubocop#4222 for a conversation about this same issue, which triggered a change to the default rubocop configuration.

- With our current config, we get a lot of linter errors from code
  that looks like this:
    expect { ... }.to change { MyModel.count }
  The linter error looks like this:
    Parenthesize the param `change { MyModel.count }` to make sure
    that the block will be associated with the `change` method call.
  This is rspec syntax and should not be flagged as a linter issue.
- See rubocop/rubocop#4222
@pk-nb
Copy link
Contributor

pk-nb commented Apr 30, 2018

good time to change, maybe with a batch for the other items

@bwebster
Copy link
Contributor Author

Yes, I figured we can include this with all the other changes. Should I merge now or let you manage that?

@pk-nb
Copy link
Contributor

pk-nb commented Apr 30, 2018

Let's hold off on merge until Thursday. I might ask everyone who has a change to help champion copying configs

@derekclee
Copy link
Contributor

I think I ran into this before so nice to fix it.

@pk-nb pk-nb merged commit 58d2f20 into master May 11, 2018
@pk-nb pk-nb deleted the blocks_in_rspec branch May 11, 2018 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants