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

Integrate RuboCop Performance #4099

Closed
Garfield96 opened this issue Mar 15, 2023 · 3 comments · Fixed by #4201 or #4210
Closed

Integrate RuboCop Performance #4099

Garfield96 opened this issue Mar 15, 2023 · 3 comments · Fixed by #4201 or #4210
Labels
feature request *Deprecated Label* Use enhancement label in general

Comments

@Garfield96
Copy link
Contributor

Is your feature request related to a problem? Please describe.

There are a few code fragments that could be improved in terms of performance and readability. E.g. some tests use select...size instead of just count or select...first instead of find. Many of these issues can be automatically detected and often even fixed by RuboCop Performance (extension of Rubocop).

Describe the solution you'd like

Fix all issues discovered by RuboCop Performance and integrate it into the CI. It most likely is not favorable to run RuboCop with all checks enabled since the style checks would require a complete linting of the project which breaks git blame. Running only the performance checks ensures that the number of changes is manageable (~180 changes).

Describe alternatives you've considered

There are other linters with a focus on performance, e.g. fasterer, but RuboCop Performance provides the advantage that it can automatically correct many issues.

Additional context

I can take over this task, if it gets accepted.

@github-actions
Copy link

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 7 days

@github-actions github-actions bot added the stale label Apr 15, 2023
@ashie ashie added feature request *Deprecated Label* Use enhancement label in general and removed stale labels Apr 17, 2023
@Garfield96
Copy link
Contributor Author

Hi @ashie,
What is your opinion on this? Shall I implement it?

@kenhys
Copy link
Contributor

kenhys commented May 1, 2023

Starting with a minimal cop may be useful, but I don't know whether the improvements may vary on supported ruby-version.
Instead of integrating CI directly, creating effective proven PR may be reasonable. (only fixing Performance/Count in one PR or so)

P.S. After merged such a PR, integrating small cop rule into CI (as a next step) is good way to keep it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request *Deprecated Label* Use enhancement label in general
Projects
None yet
4 participants