RuboCop - enable new cops, address new issues #1220
Conversation
- default to enabling new cops as they are released - address all current issues with cops we didn't previously have enabled - disable certain cops and place TODO reminders alongside them for future enabling
There was a problem hiding this comment.
Awesome proactivity! Thanks for taking this on, James!
Also, branch ref? https://robocop.fandom.com/wiki/Clara_Murphy
| @@ -13,4 +13,5 @@ Gem::Specification.new do |spec| | |||
| f.match(%r{^(test|spec|features)/}) | |||
| end | |||
| spec.require_paths = ["lib"] | |||
| spec.metadata['rubygems_mfa_required'] = 'true' | |||
There was a problem hiding this comment.
I'm curious -- why add this here?
Also, have you done any absorbing of this gem? I'm somewhat skeptical about its contemporary relevance.
There was a problem hiding this comment.
There's a new cop, Gemspec/RequireMFA, that insists on this line being present in gemspec files. I don't know if we're ready to start using that declaration yet for our 2 main gemspec files, so I have disabled the cop for now and left those files untouched. But for this file that is basically a fixture file, I figured it was ok to leave the rubocop -a driven addition.
|
Whoops, approved and then noticed the failures. My bad. |
Yep! I was referring to RoboCop's last name, shared with his wife Clara. |
`Gemspec/DateAssignment` -> `Gemspec/DeprecatedAttributeAssignment`
bring compliance with the latest RuboCop v1.31.0 cops and defaults
update the metric data test to reflect the change made to MetricData.hash
| @@ -33,7 +33,7 @@ def metric_spec= new_spec | |||
| end | |||
|
|
|||
| def hash | |||
| metric_spec.hash ^ stats.hash | |||
| [metric_spec, stats].hash | |||
There was a problem hiding this comment.
| @@ -11,8 +11,8 @@ | |||
|
|
|||
| executes do | |||
| ::NewRelic::Agent.logger.info 'Installing Rails Sunspot instrumentation' | |||
| deprecation_msg = 'The instrumentation for Sunspot is deprecated.' \ | |||
| ' It will be removed in version 9.0.0.' \ | |||
There was a problem hiding this comment.
My guess is that this trailing slash would cause issues should it ever get executed. Perhaps we've broken support for Sunspot earlier than expected?
There was a problem hiding this comment.
Nice catch! I guess that makes it two public-facing changes this release 😉
prevent rack-test 2+ from being installed with the Rails multiverse suite for now
`Gemspec/DateAssignment` -> `Gemspec/DeprecatedAttributeAssignment`
for JRuby's sake, leave ENV['HOME'] as-is and don't replace it with Dir.home
use libcurl4-openssl-dev to fetch libcurl
to make the local machine aware of which packages exist before attempting to fetch them, run `apt-get update` first. this will hopefully address the issue with Azure aggressively keeping only the very latest versions of some packages (libcurl is one that's known so far).
Now that the libcurl issue has been traced to `apt-get update` needing to be ran, go back to using the `libcurl4-nss-dev` package
CI: be sure to run `apt-get update` before `apt-get install`
| @@ -11,8 +11,8 @@ | |||
|
|
|||
| executes do | |||
| ::NewRelic::Agent.logger.info 'Installing Rails Sunspot instrumentation' | |||
| deprecation_msg = 'The instrumentation for Sunspot is deprecated.' \ | |||
| ' It will be removed in version 9.0.0.' \ | |||
There was a problem hiding this comment.
Nice catch! I guess that makes it two public-facing changes this release 😉
'enalbe' -> 'enable' Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com>
- disable `Style/MapToHash` with a reminder to enable it in a Ruby 2.6+ world - update new TODO comments with a `UNIT TESTS` label for easier categorization
future enabling