-
-
Notifications
You must be signed in to change notification settings - Fork 12
style: autofix "ALL" ruff rules + S110 #671
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would like to not have RET505/506 enabled, nor the contextlib.suppress rule.
"""If `content` is Python/REPL code, return instructions on using code blocks.""" | ||
log.trace("Creating instructions for a missing code block.") | ||
|
||
if _parsing.is_python_code(content): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be out of scope but this method should be inverted. Check for not python code and then log and return.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I don't see a line enabling ALL
rules?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I locally set it, ran stable fixes, and then reverted. The repo obviously doesn't pass ALL
out of the bot. I planned to break it out into stable / easy fixes and then slowly provide the opinionated - not automatic - fixes.
These are only fixes from stable auto fixes. Many errors still remain before ALL rules can be enabled
This PR first re-enables S110. And then I locally set the rules to "ALL" and autofixed rules.
You will notice that "COM" is ignored even though it isn't enabled. This is preemptive, but
COM
has conflicts with other format rules and IMO should be disabled from the ALL list.Note
This PR was pretty easy to make. So don't feel bad if you disagree with my direction and want to shut this down.
I have checked all the changes, and they all seem logical to me.