Conversation
streeve
left a comment
There was a problem hiding this comment.
I'm seeing that ruff is better at finding unused variables, prefers explicit public module listing, and has a very slightly different formatting default
- change in spacing rules in 2024 leads to the large number of changes, which is expected and one of the reasons I had pinned the black version to before this change (see astral-sh/ruff#8283)
- Also clarifies syntax with None value handling in Component
For what it's worth, pylint also found those issues, but it doesn't seem to weigh them highly when it comes up with its numerical score of code quality, so they tend to make it through the quality threshold check. For example, fixing all of the Ruff linting issues only moved the code quality from a pylint 8.23/10 to 8.49/10. Because Ruff is fast, I find it nicer to work with as an IDE extension to catch simple mistakes. |
streeve
left a comment
There was a problem hiding this comment.
It's not obvious to me that the ruff checks will actually cause CI failure when the checks fail (one would hope, but I've made the mistake of assuming before)
This replaces
blackautoformatting withruffautoformatting and linting.The autoformatter is the same as black, but it is more straightforward to use a single tool.
The ruff linter's static analysis is very fast, but limited in scope of what it checks compared to
pylint. I have added the ruff linting as a pre-commit check and CI step to maintain the basic quality of the code. This PR also fixes all of the ruff linter's warnings so that it passes this new check.The pylint CI step also still runs with the same behavior. It requires code to not significantly decrease the overall quality of the code beyond the current threshold, but it does not fail if pylint warnings are found.