-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
Update rubocop and enable it in CI #114
Conversation
@ShimShtein This also enables the rubocop in the CI |
.github/workflows/ci.yml
Outdated
@@ -32,3 +31,23 @@ jobs: | |||
bundler-cache: true # runs 'bundle install' and caches installed gems automatically | |||
- name: Run tests | |||
run: bundle exec rake | |||
|
|||
lint: |
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.
For performance sake, can we add rubocop just as a step before the tests? It would spare a full spin up of a machine just for running rubocop.
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.
Yes that makes sense.
fog-ovirt.gemspec
Outdated
@@ -30,6 +29,7 @@ Gem::Specification.new do |spec| | |||
spec.add_development_dependency "bundler" | |||
spec.add_development_dependency "pry" | |||
spec.add_development_dependency "rake" | |||
spec.add_development_dependency "rubocop", "~> 0.52" | |||
spec.add_development_dependency "rubocop" |
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.
Let's freeze rubocop version. It tends to change under our feet and fail CI.
If you think we would like to keep rubocop up to date, I would recommend adding a separate CI job that will test against the latest rubocop once in a while and we will monitor it.
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.
Fixed the version, we could use dependabot for 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.
I don't have experience with dependabot, but I am not against it. We can always disable it later 🤷
@dosas I see rubocop started to fail in CI. Any idea why? |
No description provided.