-
Notifications
You must be signed in to change notification settings - Fork 248
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
[lint] uses ruff to format instead of black #14131
Conversation
c5371ef
to
9abe74e
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.
Thanks for doing this! I'm excited to have lightning fast linting and formatting for the whole codebase.
It's a little unclear to me though whether this is meant to tackle formatting and format-checking with ruff
or adding hail
to ruff's linting.
@@ -39,10 +39,8 @@ check-all: check-hail check-services | |||
|
|||
.PHONY: check-hail-fast | |||
check-hail-fast: | |||
ruff check hail/python/hail | |||
ruff check hail/python/hailtop | |||
ruff check hail |
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 think ruff check
exclusively refers to the ruff linter. I think to check formatting you need ruff format --check
.
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.
ohhhhhh thank you! i will take another look with this in mind
"PLC0132", # `TypeVar` name does not match assigned variable name | ||
"PLE0307", # `__str__` does not return `str` | ||
"PLC0414", # Import alias does not rename original package | ||
"PLW0127", # Self-assignment of variable |
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 am all for these changes, but IIUC these are related to expanding the scope of ruff as a linter to include the hail
directory, and not necessarily to do with replacing black as a formatter. I'm happy to review a big PR that includes linting fixes, just as long as it doesn't include auto-formatting changes.
okay @daniel-goldstein #14132 replaces just the formatting/format checking with |
As of #14119, we lint and format using
black
; this commit usesruff
instead, as it is faster and is intended to be a drop-in replacement forblack
. There are some rules thatruff
cares about thatblack
doesn't, which would require some dramatic reformats to the codebase, so for the time being, these are added to the list of rules to ignore, and can be added back in all at once or progressively later on.