-
Notifications
You must be signed in to change notification settings - Fork 11
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
Switch from black
to ruff format
#4559
Conversation
It gives a warning when running the Ruff formatter. This rule can cause the linter and formatter to conflict, meaning you need to rerun `ruff check --fix` again after `ruff format`. See astral-sh/ruff#8272 (comment)
`outputs/` was the only Black exclusion not being excluded for Ruff, so also add that exclusion there.
This also removes several transitive dependencies.
ea905d0
to
79eb8a5
Compare
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.
Couple of questions, but otherwise 🚀
$BIN/ruff check --fix . | ||
$BIN/ruff format . |
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.
What does check --fix
do that format
doesn't do?
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.
ruff check
runs the linter and fixes any automatically fixable issues it finds (example: removing unused imports), but ruff check
doesn't run the formatter.
It's planned for ruff
to eventually unify the two, which will reduce this to a single command instead.
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 need to read up on the difference between linting and formatting!
@@ -121,6 +109,7 @@ exclude = [ | |||
"docs", | |||
"htmlcov", | |||
"node_modules", | |||
"outputs", |
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.
And static
?
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.
Unless I've missed something, static
is already included below.
outputs
was the only entry for the Black excludes, that wasn't listed already in Ruff's excludes.
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.
My error. Sorry!
Fixes #4542.
ruff format
is considerably faster thanblack
, while maintaining almost entirely the same formatting output. After deleting the cache for both and running against job-server commit 241415f:(Note: the Ruff warning is fixed in this PR; it's because the configuration's not updated on the
main
branch where I ran the above.)As a bonus, this also removes several additional development dependencies.