Skip to content
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

Rubocops compatible with Ruby 2.4+ #1840

Merged
merged 14 commits into from
Mar 1, 2023
Merged

Rubocops compatible with Ruby 2.4+ #1840

merged 14 commits into from
Mar 1, 2023

Conversation

kaylareopelle
Copy link
Contributor

@kaylareopelle kaylareopelle commented Feb 24, 2023

Overview

Our agent's previous support of Ruby 2.2 and 2.3 prevented some Rubocop linters from being enabled. This PR enables the following:

It disables Style/FetchEnvVar. When ENV.fetch fails, a KeyError or the explicitly specified default
value is returned. We don't want to raise an unexpected error in a customer's environment. Also, fetch is slightly less performant than ENV[] when there is not a non-nil replacement value.

Resolves: #1787
Resolves: #1246

When FetchEnvVar fails, a KeyError or the explicitly specified default
value is returned.

We don't want to raise an unexpected error in a customer's environment.

Also, fetch is slightly less performant than ENV[]
@kaylareopelle kaylareopelle marked this pull request as ready for review February 25, 2023 00:01
@fallwith
Copy link
Contributor

@kaylareopelle perhaps it's an issue best left for a separate PR, but my preference would be that for everywhere we now plan to align with the defaults, we would simply remove the Enabled: false based override from .rubocop_todo.yml without actually redundantly adding a new Enabled: true section to .rubocop.yml.

@kaylareopelle
Copy link
Contributor Author

@kaylareopelle perhaps it's an issue best left for a separate PR, but my preference would be that for everywhere we now plan to align with the defaults, we would simply remove the Enabled: false based override from .rubocop_todo.yml without actually redundantly adding a new Enabled: true section to .rubocop.yml.

I'm comfortable with that change. I'm not sure if all of these are on by default. I'll check Rubocop and adjust accordingly. I think makes sense to add another issue to adjust the other cops, since some of the enabled cops may be disabled by default.

@kaylareopelle
Copy link
Contributor Author

@fallwith - RedundantStringEscape and Sum are pending, so I left those references in to make sure they get applied. The others were removed in 03d82e6

@@ -128,7 +128,7 @@ def self.parse_cpuinfo(cpuinfo)

num_physical_packages = cores.keys.map(&:first).uniq.size
num_physical_cores = cores.size
num_logical_processors = cores.values.reduce(0, :+)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This cop is unsafe if sum is called on an object that doesn't have a sum method, such as a String. cores.values should always be an array of integers based on the default values of the Hash defined on line 110

lib/new_relic/agent.rb Show resolved Hide resolved
lib/new_relic/agent.rb Show resolved Hide resolved
lib/new_relic/agent/heap.rb Show resolved Hide resolved
lib/new_relic/cli/commands/deployments.rb Show resolved Hide resolved
lib/new_relic/dependency_detection.rb Show resolved Hide resolved
lib/new_relic/helper.rb Show resolved Hide resolved
lib/new_relic/rack/browser_monitoring.rb Show resolved Hide resolved
fallwith
fallwith previously approved these changes Feb 28, 2023
infinite_tracing/lib/infinite_tracing/config.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@tannalynn tannalynn left a comment

Choose a reason for hiding this comment

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

🎉

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2023

SimpleCov Report

Coverage Threshold
Line 93.96% 93%
Branch 85.49% 84%

@kaylareopelle kaylareopelle merged commit d8107d4 into dev Mar 1, 2023
@kaylareopelle kaylareopelle deleted the rubocop-ruby24-compat branch March 1, 2023 19:10
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.

Review rubocops that became available with 9.0.0 Replace String.new() with +string
3 participants