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 to rubocop 1.13.0 and re-enable Bundler/GemComment
.
#44
Conversation
@@ -1,3 +1,7 @@ | |||
require: | |||
- rubocop-performance | |||
- rubocop-rails |
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.
Why did we need this change? If we do need it, should the rubocop-rails require go in rubocop_rails.yml?
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.
@jturkel The need for this is that otherwise we get these errors:
As for moving require: rubocop-rails
to rubocop_rails.yml
, the issue is that this file has the following lines:
salsify_rubocop/conf/rubocop_without_rspec.yml
Lines 192 to 193 in 8dd2369
Rails: | |
Enabled: false |
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 guess this gem depends on rubocop-rails even if the project isn't using Rails so this should be innocuous.
8dd2369
to
bbd011e
Compare
Wow, took me quite a while to realize that the tests themselves had suffered breaking changes and had to be updated in conjunction with our custom cops! Not a fun time, I was getting very frustrated trying to fix the custom cops and the error messages weren't helpful at all. Anyway, it was documented: https://docs.rubocop.org/rubocop/v1_upgrade_notes.html#upgrading-specs |
Bundler/GemComment
cop.Bundler/GemComment
.
.rubocop.yml
Outdated
@@ -2,7 +2,7 @@ inherit_from: | |||
- conf/rubocop.yml | |||
|
|||
AllCops: | |||
TargetRubyVersion: 2.4 | |||
TargetRubyVersion: 2.5 |
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.
Should be bump this (and the minimum Ruby version in the gemspec) to Ruby 2.6 since Ruby 2.5 is EOL now?
@@ -1,3 +1,7 @@ | |||
require: | |||
- rubocop-performance | |||
- rubocop-rails |
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 guess this gem depends on rubocop-rails even if the project isn't using Rails so this should be innocuous.
694af55
to
75c1c64
Compare
(as part of this jira ticket)
Follow up to #41, as rubocop/rubocop#9358 was merged and released as part of rubocop 1.13.0.
Not that I agree that
>= x
version specifiers don't deserve an explanation (someone might want to know, if they'd like to downgrade for another reason) but I'm not dying on this hill 😛(Meant for https://github.com/salsify/con-u/pull/1752)
prime: @jturkel
cc: @erikkessler1, @donbonifacio, @beardman