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

Avoid running logical line rule logic if not enabled #11951

Merged
merged 2 commits into from
Jun 20, 2024

Conversation

dhruvmanila
Copy link
Member

Summary

This PR updates the logical line rules entry-point function to only run the logic if any of the rules within that group is enabled.

Although this shouldn't really give any performance improvements, it's better not to do additional work if we can. This is also consistent with how other rules are run.

Test Plan

cargo insta test

@dhruvmanila dhruvmanila added the internal An internal refactor or improvement label Jun 20, 2024
Copy link
Contributor

github-actions bot commented Jun 20, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. This might improve performance, it just won't show in our benchmarks because we either run all or no logical line rules.

crates/ruff_linter/src/checkers/logical_lines.rs Outdated Show resolved Hide resolved
@dhruvmanila dhruvmanila enabled auto-merge (squash) June 20, 2024 16:25
@dhruvmanila dhruvmanila merged commit 3f884b4 into main Jun 20, 2024
18 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/logical-line-rules branch June 20, 2024 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants