-
-
Notifications
You must be signed in to change notification settings - Fork 358
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
Setup rubocop. #770
Setup rubocop. #770
Conversation
This is green and I'd like to include this in 3.1...otherwise we're liable to have merge conflicts when backporting bug fixes from master to 3-1-maintenance. Anyone want to review this? /cc @JonRowe @samphippen @soulcutter @cupakromer @xaviershay @yujinakayama |
desc 'Run RuboCop on the lib directory' | ||
RuboCop::RakeTask.new(:rubocop) do |task| | ||
task.patterns = ['lib/**/*.rb'] | ||
end |
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.
This probably needs to be scoped to if rubocop
is available. Otherwise, on other ruby versions it will cause any rake command to fail.
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.
That's probably a good idea, but it hasn't been a problem in the other repos:
- https://github.com/rspec/rspec-expectations/blob/318cda1ba796ee0882e9665ff6a7b116e118b0a7/Rakefile#L74-L78
- https://github.com/rspec/rspec-support/blob/master/Rakefile#L26-L30
...and it hasn't been a problem yet. Still, it's a good idea.
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.
Actually, I'm going to nuke the task entirely; the rubocop
command is more flexible and we don't need this for our build.
In general LGTM. Left a few comments / questions. None are merge blockers. |
👍 merge when green |
@@ -250,15 +253,14 @@ def allow_no_prepended_module_definition_of(method_name) | |||
return unless problem_mod | |||
|
|||
raise RSpec::Mocks::MockExpectationError, | |||
"Using `any_instance` to stub a method (#{method_name}) that has been " + | |||
"Using `any_instance` to stub a method (#{method_name}) that has been " \ | |||
"defined on a prepended module (#{problem_mod}) is not supported." |
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 this line should be aligned with the previous line (and maybe RuboCop should be improved to detect this).
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.
Yeah woops, that's weird. I'll fix that.
Closes #702.
03717e6
to
a1b93d0
Compare
Closes #702.