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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃毃 Add rubocop-performance linting #1196

Closed

Conversation

JerrodCarpenter
Copy link
Contributor

This may be a tad presumptuous.. but I thought it might be nice to add rubocop-performance for the following reasons:

  • The gem is marshaling a lot of data and there are risks of us adding some inefficiencies as we add new commands.
  • Increasing confidence when reviewing PRs.

Really, I know I may be over stepping here, no worries if I'm way off track.

@byroot
Copy link
Collaborator

byroot commented May 30, 2023

My problem here is mostly than many cops in rubocop-performance are a bit questionable. Some were added a long time ago based on questionable micro benchmarks or weren't re-considered on more modern rubies.

That said there may be value here, so I'm OK with the general idea, but I'll have to review the cops, and I think I'll disable it for the test directory.

Also the correction it did broke CI.

@JerrodCarpenter
Copy link
Contributor Author

My problem here is mostly than many cops in rubocop-performance are a bit questionable. Some were added a long time ago based on questionable micro benchmarks or weren't re-considered on more modern rubies.

hmmm fair enough

That said there may be value here, so I'm OK with the general idea, but I'll have to review the cops, and I think I'll disable it for the test directory.

Also the correction it did broke CI.

I'll take a look at CI, might be a little while if that's okay.

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.

None yet

2 participants