Add RuboCop support #2894
Merged
Add RuboCop support #2894
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hmm. I'm clearly misunderstanding how to get RuboCop into the Gemfile, but I don't get why what I've got isn't working. (Works locally, fails on Travis.) |
varyonic
reviewed
Jun 19, 2018
Gemfile
Outdated
| @@ -2,6 +2,7 @@ source 'https://rubygems.org' | |||
| gemspec | |||
|
|
|||
| gem 'jruby-openssl', :platforms => :jruby | |||
| gem 'rubocop', '~> 0.57.2', require: false | |||
There was a problem hiding this comment.
We don't require: false, eg. activeadmin/activeadmin@508287f
There was a problem hiding this comment.
I dropped the require: false and it still dies.
varyonic
reviewed
Jun 20, 2018
| task :test => 'test:units' | ||
|
|
||
| RuboCop::RakeTask.new |
There was a problem hiding this comment.
Looks good to me. FWIW we went with https://github.com/activeadmin/activeadmin/pull/5174/files
a38f8dc to
9710535
Compare
|
@activemerchant/spreedly-runtime This is ready for an approval/rejection at this point; it's passing CI and clean to merge. |
This introduces RuboCop to Active Merchant, defaulting to the normal Ruby community guidelines. To grandfather in our existing code, I've pre-generated a `.rubocop_todo.yml` file, which will allow gradual migration. To run RuboCop, you can run `rake rubocop`, but I've also introduced a test:local Rake target that runs both the unit tests and RuboCop checks. I've made this be the default Rake task as well, which means Travis will (if this PR lands) require RuboCop passing for test approval. As a proof-of-concept, I went ahead and fixed WhenThen and YodaConditional style violations, which had sufficiently few violations to work for demo purposes. All PayPal-related code is excluded because it crashes RuboCop. Unit: 3864 tests, 67919 assertions, 0 failures, 0 errors, 0 pendings, 2 omissions, 0 notifications 100% passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This introduces RuboCop to Active Merchant, defaulting to the normal
Ruby community guidelines. To grandfather in our existing code, I've
pre-generated a
.rubocop_todo.ymlfile, which will allowgradual migration.
To run RuboCop, you can run
rake rubocop, but I've also introduced atest:local Rake target that runs both the unit tests and RuboCop
checks. I've made this be the default Rake task as well, which means
Travis will (if this PR lands) require RuboCop passing for test
approval.
As a proof-of-concept, I went ahead and fixed WhenThen and
YodaConditional style violations, which had sufficiently few violations
to work for demo purposes.
All PayPal-related code is excluded because it crashes RuboCop.
Unit: 3864 tests, 67919 assertions, 0 failures, 0 errors, 0 pendings, 2 omissions, 0 notifications
100% passed