Skip to content

(maint) Add rubocop and allow failures in Travis CI#183

Merged
mckern merged 2 commits into
puppetlabs:masterfrom
glennsarti:add-rubocop
Feb 22, 2017
Merged

(maint) Add rubocop and allow failures in Travis CI#183
mckern merged 2 commits into
puppetlabs:masterfrom
glennsarti:add-rubocop

Conversation

@glennsarti

Copy link
Copy Markdown
Contributor

This PR adds the rubocop gem, a rubocop rake task and an initial rubocop
configuration file in prepartion for using rubocop in CI.

This PR adds rubocop checks to Travis for PR CI. Any rubocop failures will
not fail the build yet. Fixes for rubocop violations will happen in later
commits.

Comment thread Rakefile
cli = RuboCop::CLI.new
exit_code = cli.run(%w(--display-cop-names --format simple))
raise "RuboCop detected offenses" if exit_code != 0
end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if I told you that Rubocop has Rake shims to help you out here?

From https://github.com/mckern/vanagon/blob/master/Rakefile:

desc 'Run RuboCop'
RuboCop::RakeTask.new(:rubocop) do |task|
  task.options << '--display-cop-names'
end

That'll give you:

mckern@flexo vanagon (git:master) $ bundle exec rake -T | grep rubo
rake rubocop               # Run RuboCop
rake rubocop:auto_correct  # Auto-correct RuboCop offenses
mckern@flexo vanagon (git:master) $

Comment thread Gemfile Outdated
gem 'rspec', '>= 3.2'
gem 'simplecov', '>= 0.11.2'
gem 'yarjuf', '>= 2.0'
gem "rubocop", :platforms => [:ruby]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment about Rubocop being A-OK if we transition to Jruby 9000 but for now it's MRI only?

@mckern

mckern commented Feb 9, 2017

Copy link
Copy Markdown
Contributor

Here's a neat Rubocop trick that can help keep .rubocop.yml reasonably small. Run rubocop --auto-gen-config --exclude-limit <some number>, and you'll get a pretty neat breakdown of where your problems are and how to make Rubocop ignore them for now:

28 files inspected, 1534 offenses detected
Created .rubocop_todo.yml.
Run `rubocop --config .rubocop_todo.yml`, or add `inherit_from: .rubocop_todo.yml` in a .rubocop.yml file.
mckern@flexo vmpooler (git:master) $ rubocop --auto-gen-config --exclude-limit 100
<--- snip a LOT of warnings and failures --->
Run `rubocop --config .rubocop_todo.yml`, or add `inherit_from: .rubocop_todo.yml` in a .rubocop.yml file.
mckern@flexo vmpooler (git:master) $ rubocop --config .rubocop_todo.yml
Inspecting 28 files
..............C.............

Offenses:

lib/vmpooler/vsphere_helper.rb:398:9: C: Use the return of the conditional for variable assignment and comparison.
        if child.name == snapshotname ...
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

28 files inspected, 1 offense detected
mckern@flexo vmpooler (git:master) $

37900128

@mckern

mckern commented Feb 9, 2017

Copy link
Copy Markdown
Contributor

@glennsarti you said I've got permissions to push up to your fork/branch? I might do so if you don't mind.

@glennsarti

Copy link
Copy Markdown
Contributor Author

@mckern Yes. by default (for a while now) all maintainers get write rights on the originating PR branches in the other person's repo (That's the Allow edits from maintainers. tick box). Feel free to rebase/add etc.

This commit adds the rubocop gem, a rubocop rake task and an initial rubocop
configuration file in prepartion for using rubocop in CI.

This commit also adds a rubocop todo file that exempts existing files from
violations.
This commit adds rubocop checks to Travis for PR CI.  Any rubocop failures will
not fail the build yet.  Fixes for rubocop violations will happen in later
commits.
@glennsarti

Copy link
Copy Markdown
Contributor Author

Updated as per suggestions...but Travis seems to not be running on this PR :-(

@stahnma

stahnma commented Feb 21, 2017

Copy link
Copy Markdown

@glennsarti do you want this one in before the test additions? If so, merge it on in.

@glennsarti

Copy link
Copy Markdown
Contributor Author

Rubocop excludes specs dir so order should not matter. I'm generally against merging my own PRs but if +1 it, I'm happy.

@mckern

mckern commented Feb 22, 2017

Copy link
Copy Markdown
Contributor

Yeah, don't merge your own PRs.

@mckern mckern merged commit 145ecb4 into puppetlabs:master Feb 22, 2017
@glennsarti glennsarti deleted the add-rubocop branch March 1, 2017 23:06
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