Updates Rubocop Gems #1254
Updates Rubocop Gems #1254
Conversation
|
Hi @aried3r, I created a PR to set the RuboCop Performance version so we stop introducing unexpected offenses in CI. What do you think? |
|
I like it! It's there a reason you used 1.6.1 instead of 1.7.1? |
|
Maybe we should to the same for |
tlynam
force-pushed
the
set-rubocop-performance-minor-version
branch
2 times, most recently
from
53e8930
to
abc82a6
Compare
lib/paper_trail/events/base.rb
Outdated
| @@ -218,7 +218,7 @@ def notable_changes | |||
| end | |||
|
|
|||
| # @api private | |||
| def notably_changed | |||
| def notably_changed # rubocop:disable Metrics/CyclomaticComplexity | |||
There was a problem hiding this comment.
Rubocop reported this method but it appears too complex to refactor for this PR. What do you think of disabling for this method?
There was a problem hiding this comment.
I assume this is because of recent changes in rubocop, see here: rubocop/rubocop#8149.
They also bumped the default from 6 to 7.
paper_trail does set a lower goal. Do you by any chance remember how high complexity was? But I agree this is too much for this PR. But rather than disabling it, I'd increase it in .rubocop_todo.yml.
Lines 12 to 13 in fa8b1b2
I'm a bit hesitant to change it, since @jaredbeck has set these rules and goals. WDYT, @jaredbeck?
| @@ -2,5 +2,5 @@ | |||
|
|
|||
| # This file is used by Rack-based servers to start the application. | |||
|
|
|||
| require ::File.expand_path("../config/environment", __FILE__) | |||
| require ::File.expand_path("config/environment", __dir__) | |||
There was a problem hiding this comment.
Rubocop reported this. I'm not familiar with this but I'm assuming it's a safe change?
| @@ -12,7 +12,7 @@ module PaperTrail | |||
| specify { expect(PaperTrail.serializer).to be PaperTrail::Serializers::YAML } | |||
|
|
|||
| it "store out as a plain hash" do | |||
| expect(value =~ /HashWithIndifferentAccess/).to be_nil | |||
| expect(value).not_to include("HashWithIndifferentAccess") | |||
There was a problem hiding this comment.
Rubocop reported and it's easy to fix
|
Hi @aried3r, that sounds good. Initially was trying to making the smallest change that resolved it but how about we update all the rubocop gems? What do you think? |
tlynam
changed the title
I'm 100% in favor of locking down development gems to make CI more reliable. Nice work!
Since we haven't achieved 6 yet, this is convenient for us 😆 . Seriously, I don't see a big difference between 6 and 7 and I'm happy either way, thanks! |
|
Alright, then I think we should set the internal goal to 7 (RuboCop's new default) and the Since by now rubocop 0.89.0 has been released, I'd like to get this shipped soon. My apologies for the slow replies. @tlynam, feel free to leave it at 0.88.0 or update to 0.89.0 if you see fit. Thanks! |
tlynam
force-pushed
the
set-rubocop-performance-minor-version
branch
from
abc82a6
to
6f83983
Compare
- Manually specifies minor version so it doesn't automatically update minor verison during CI tests. - Updates other rubocop gems. - Resolves cops
tlynam
force-pushed
the
set-rubocop-performance-minor-version
branch
from
6f83983
to
1987422
Compare
|
Hi @aried3r, what do you think of keeping it at |
Thank you for your contribution!
Check the following boxes:
master(if not - rebase it).code introduces user-observable changes.
and description in grammatically correct, complete sentences.