Skip to content

Freeze rubocop version for stable CI#278

Merged
LeFnord merged 5 commits intoruby-grape:masterfrom
kachick:fix-rubocop
Oct 31, 2017
Merged

Freeze rubocop version for stable CI#278
LeFnord merged 5 commits intoruby-grape:masterfrom
kachick:fix-rubocop

Conversation

@kachick
Copy link
Copy Markdown
Contributor

@kachick kachick commented Oct 31, 2017

@grape-bot
Copy link
Copy Markdown

grape-bot commented Oct 31, 2017

1 Warning
⚠️ Unless you’re refactoring existing code, please update CHANGELOG.md.
1 Message
📖 We really appreciate pull requests that demonstrate issues, even without a fix. That said, the next step is to try and fix the failing tests!

Here's an example of a CHANGELOG.md entry:

* [#278](https://github.com/ruby-grape/grape-entity/pull/278): Freeze rubocop version for stable ci - [@kachick](https://github.com/kachick).

Generated by 🚫 danger

@coveralls
Copy link
Copy Markdown

coveralls commented Oct 31, 2017

Coverage Status

Coverage remained the same at 92.72% when pulling a1fcb44 on kachick:fix-rubocop into 6a384cd on ruby-grape:master.

@LeFnord
Copy link
Copy Markdown
Member

LeFnord commented Oct 31, 2017

thanks @kachick … did you tested it with #277 and it is green?

Comment thread grape-entity.gemspec Outdated
s.add_development_dependency 'bundler'
s.add_development_dependency 'rake'
s.add_development_dependency 'rubocop', '~> 0.48'
s.add_development_dependency 'rubocop', '~> 0.48.0'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please also add require: false

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So we need to add the line into Gemfile instead of gemspec, I think 🤔

Could you check 8895a69 ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

And added c362d74 🙇

Copy link
Copy Markdown
Member

@LeFnord LeFnord Oct 31, 2017

Choose a reason for hiding this comment

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

think you can remove this line

@coveralls
Copy link
Copy Markdown

coveralls commented Oct 31, 2017

Coverage Status

Coverage remained the same at 92.72% when pulling 8895a69 on kachick:fix-rubocop into 6a384cd on ruby-grape:master.

Comment thread Gemfile
gemspec

group :development do
group :development, :test do
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it adds always 0.51 → maybe you have to remove the line in gem spec?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it adds always 0.51

Yes, How about b66105f?

Comment thread Gemfile
gemspec

group :development, :test do
gem 'rubocop', '~> 0.48.0', require: false
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

Comment thread grape-entity.gemspec Outdated
s.add_development_dependency 'bundler'
s.add_development_dependency 'rake'
s.add_development_dependency 'rubocop', '~> 0.48'
s.add_development_dependency 'rubocop', '~> 0.48.0'
Copy link
Copy Markdown
Member

@LeFnord LeFnord Oct 31, 2017

Choose a reason for hiding this comment

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

think you can remove this line

@kachick
Copy link
Copy Markdown
Contributor Author

kachick commented Oct 31, 2017

#278 (comment)

did you tested it with #277 and it is green?

Yes, I tested in my local. But we added some changes. So I'll ensure in #279 💪

@coveralls
Copy link
Copy Markdown

coveralls commented Oct 31, 2017

Coverage Status

Coverage remained the same at 92.72% when pulling c362d74 on kachick:fix-rubocop into 6a384cd on ruby-grape:master.

@LeFnord
Copy link
Copy Markdown
Member

LeFnord commented Oct 31, 2017

I see … cool 👍

@LeFnord
Copy link
Copy Markdown
Member

LeFnord commented Oct 31, 2017

please can you close the other PRs, think here is all in 😉

@kachick
Copy link
Copy Markdown
Contributor Author

kachick commented Oct 31, 2017

Thank you! Then, Can I close #279 ? ⏹

@coveralls
Copy link
Copy Markdown

coveralls commented Oct 31, 2017

Coverage Status

Coverage remained the same at 92.72% when pulling b66105f on kachick:fix-rubocop into 6a384cd on ruby-grape:master.

@coveralls
Copy link
Copy Markdown

coveralls commented Oct 31, 2017

Coverage Status

Coverage increased (+1.8%) to 94.538% when pulling fa4081a on kachick:fix-rubocop into 6a384cd on ruby-grape:master.

@kachick
Copy link
Copy Markdown
Contributor Author

kachick commented Oct 31, 2017

Oh sorry to misunderstood 🙇

And Thank you for the review! I'll update rest PRs 💪

@kachick kachick deleted the fix-rubocop branch October 31, 2017 23:55
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.

4 participants