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

Enable Ruff RET (flake8-return) #13323

Closed
wants to merge 3 commits into from
Closed

Enable Ruff RET (flake8-return) #13323

wants to merge 3 commits into from

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Dec 28, 2024

Ref #13295
https://docs.astral.sh/ruff/rules/#flake8-return-ret

Everything is autofixable

superfluous-else-* (RET505-RET508)'s "make the code more readable" is arguable if you find the else keyword to be helpful to your flow comprehension.
I like its reduced nesting for early conditional guards, but it incidentally catches more than that. Maybe if it could be configured to not apply to single statements ? (astral-sh/ruff#15167)

I find the rest of the RET rules to be good sanity checks.

@Avasam Avasam changed the title Enable Ruff flake8-return Enable Ruff RET (flake8-return) Dec 28, 2024
@JelleZijlstra
Copy link
Member

I don't find these to be improvements.

@Avasam
Copy link
Collaborator Author

Avasam commented Dec 30, 2024

I don't find these to be improvements.

Just to be clear, are you talking about the changes caused by RET505-RET508 (which I agree, shouldn't be enforced, I just disabled them, e37aa4e (#13323) can be looked at to see the difference) or the entire ruleset ?

@AlexWaygood
Copy link
Member

Hmmm... I think I'm -1 on this whole category, actually. These all seem like pretty pedantic stylistic lints that sometimes hurt readability, and usually don't significantly improve readability. (In the rare occasions where there is a significant improvement in readability, I think we'd probably just flag it in code review.)

I think overall this category would be likely just to cause us irritation rather than actually help us, so I think I'd prefer to leave these disabled

@Avasam
Copy link
Collaborator Author

Avasam commented Jan 2, 2025

Let's close this one and leave it to reviews.

@Avasam Avasam closed this Jan 2, 2025
@Avasam Avasam deleted the Ruff-RET branch January 2, 2025 22:18
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.

3 participants