Skip to content

Add rubocop to project, delivers #179 #181

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

Merged
merged 1 commit into from
Jan 7, 2015
Merged

Add rubocop to project, delivers #179 #181

merged 1 commit into from
Jan 7, 2015

Conversation

mynameisrufus
Copy link
Contributor

This adds https://github.com/bbatsov/rubocop to the project with an auto
generated .rubocop_todo.yml. The violations can be addressed over time without
breaking the build.

@@ -10,4 +13,4 @@ Rake::TestTask.new do |t|
t.verbose = true
end

task :default => :test
task default: [:rubocop, :test]
Copy link
Member

Choose a reason for hiding this comment

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

Could you change travis.yml to run these two tasks separately? I'd prefer the default rake task be just tests.

Copy link
Member

Choose a reason for hiding this comment

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

👍 agreed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMHO having it not enforced, you can just do a gem install rubocop and run rubocop, no need for this PR. See a grape build for example: https://travis-ci.org/intridea/grape/jobs/44609939

@mtodd
Copy link
Member

mtodd commented Dec 20, 2014

@mynameisrufus was hoping to see a demo of RuboCop catching/enforcing a style guide rule here since I'm not familiar with its output. I can check it out and run it locally, but will delay me getting back to this.

@mynameisrufus
Copy link
Contributor Author

This is the output with Lint/UnusedBlockArgument offences detected:

screen shot 2014-12-21 at 11 36 09 am

@jch
Copy link
Member

jch commented Dec 22, 2014

👍 for the output. @mynameisrufus once you tweak it so this only runs in Travis, I'll be happy to merge.

@mynameisrufus
Copy link
Contributor Author

Oh sorry I misunderstood, I thought you did not want it to run in travis, coming right up.

@mynameisrufus
Copy link
Contributor Author

Ready for merge

@mtodd
Copy link
Member

mtodd commented Dec 24, 2014

@mynameisrufus looks like Rubinius is failing here: https://travis-ci.org/ruby-ldap/ruby-net-ldap/jobs/44879650

This might need to be only enabled for MRI Rubies?

@mtodd
Copy link
Member

mtodd commented Dec 24, 2014

Should running rake rubocop ignore the config file? When I run that, it reports the warnings and exits with 1.

@mynameisrufus
Copy link
Contributor Author

I don't know how you are getting that to happen. Can you give me more details?

Here is what I get running rake rubocop:

❯ rake rubocop                                                                                                                                                           ruby-net-ldap/git/feature/rubocop
warning: parser/current is loading parser/ruby21, which recognizes
warning: 2.1.5-compliant syntax, but you are running 2.1.0.
Running RuboCop...
Inspecting 48 files
................................................

48 files inspected, no offenses detected
❯ echo "exit code: $?"                                                                                                                                                   ruby-net-ldap/git/feature/rubocop
exit code: 0

I'm not sure what is happening on Travis, I ran the same locally no problem:

❯ bundle exec rake rubocop test                                                                                                                                          ruby-net-ldap/git/feature/rubocop
warning: parser/current is loading parser/ruby21, which recognizes
warning: 2.1.5-compliant syntax, but you are running 2.1.0.
Running RuboCop...
Inspecting 48 files
................................................

48 files inspected, no offenses detected
/opt/boxen/rbenv/versions/rbx-2.4.1/bin/rbx -I"lib:test" -I"/Users/rufuspost/Projects/ruby/gems/ruby-net-ldap/.gems/rbx/2.1/gems/rake-10.4.2/lib" "/Users/rufuspost/Projects/ruby/gems/ruby-net-ldap/.gems/rbx/2.1/gems/rake-10.4.2/lib/rake/rake_test_loader.rb" "test/ber/core_ext/test_array.rb" "test/ber/core_ext/test_string.rb" "test/ber/test_ber.rb" "test/integration/test_add.rb" "test/integration/test_ber.rb" "test/integration/test_bind.rb" "test/integration/test_delete.rb" "test/integration/test_open.rb" "test/integration/test_return_codes.rb" "test/integration/test_search.rb" "test/test_dn.rb" "test/test_entry.rb" "test/test_filter.rb" "test/test_filter_parser.rb" "test/test_helper.rb" "test/test_ldap.rb" "test/test_ldap_connection.rb" "test/test_ldif.rb" "test/test_password.rb" "test/test_rename.rb" "test/test_search.rb" "test/test_snmp.rb" "test/test_ssl_ber.rb"
Run options:

# Running tests:

Finished tests in 0.874936s, 236.5887 tests/s, 657.1909 assertions/s.      archIntegration:0x7f84>:0x7f70>
207 tests, 575 assertions, 0 failures, 0 errors, 0 skips

ruby -v: rubinius 2.4.1 (2.1.0 b332f133 2014-12-04 3.5.0 JI) [x86_64-darwin14.0.0]

Rubocop is built against rbx-2 also but the current version might have issues https://travis-ci.org/bbatsov/rubocop/builds and rubocop/rubocop#1494

Do we not run rubocop on rbx or just leave it broken for now? It's broken sans rubocop anyway.

@mtodd
Copy link
Member

mtodd commented Dec 27, 2014

@mynameisrufus I ran bundle exec rake rubocop in my local checkout, after running bundle install, and got this: https://gist.github.com/mtodd/0c2116bb1c5f58428f64

Do we not run rubocop on rbx or just leave it broken for now? It's broken sans rubocop anyway.

It's broken for JRuby but not RBX (a known error): https://travis-ci.org/ruby-ldap/ruby-net-ldap/builds/44998776

I say we just run Rubocop for MRI. I'd have to check the Travis docs to figure out how to detect and run it only for that env.

This adds https://github.com/bbatsov/rubocop to the project with an auto
generated `.rubocop_todo.yml`. The violations can be addressed over time without
breaking the build.

Also added is the `rake ci` task that runs `test` and `rubocop` only on mri. The
`rubotest` task is the same is `rake ci` but will run on all rubies, the default
rake task just runs test as per usual.

Conflicts:
	net-ldap.gemspec
@mynameisrufus
Copy link
Contributor Author

@mtodd I have added pkg to the ignore property in .rubocop.yml so you should be good now. Also a new task has been added called rake ci that will only run RuboCop on mri. I could not find anything to run a specific task for a specific ruby in the travis docs so implemented in the Rakefile. Should be good to merge now 🔥

jch added a commit that referenced this pull request Jan 7, 2015
@jch jch merged commit 876a50e into ruby-ldap:master Jan 7, 2015
@jch
Copy link
Member

jch commented Jan 7, 2015

🍻

@jch jch mentioned this pull request Jan 7, 2015
astratto pushed a commit to astratto/ruby-net-ldap that referenced this pull request Dec 18, 2015
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