-
-
Notifications
You must be signed in to change notification settings - Fork 20
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: unify Black with Ruff #110
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.
Thanks! I pushed a few small fixes to your branch. One question:
select = [ | ||
"E", # pycodestyle errors | ||
"F", # pyflakes errors | ||
"I", # isort | ||
"ISC", # flake8-implicit-str-concat |
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 removed this since there was a CI warning saying that it might conflict with the formatting hook
@@ -445,7 +445,7 @@ def check_line_too_long(file, lines, options=None): | |||
continue # ignore anonymous hyperlink targets | |||
if _is_very_long_string_literal(line): | |||
continue # ignore a very long literal string | |||
yield lno + 1, f"Line too long ({len(line)-1}/{options.max_line_length})" | |||
yield lno + 1, f"Line too long ({len(line) - 1}/{options.max_line_length})" |
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.
This was reformatted because black can't format inside f-strings yet, whereas ruff can. Black would reformat this too if it could
@@ -4,6 +4,7 @@ | |||
- All constants are ALL_CAPS | |||
- All compiled regexes are suffixed by _RE | |||
""" | |||
|
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.
Black also does this with the latest version of black; we just had an old version of black pinned in CI
Btw, is there precommit bot enabled? |
No, we're not using pre-commit.ci right now |
Yes, I noticed, just was wondering if there is a reason not to use it? |
I'm in favour: it helps fix lint things in PRs, meaning humans can spend more time reviewing non-trivial stuff; and will send a PR (no more than quarterly) to keep the versions up-to-date. |
seconded |
Exactly my experience, now just finding admin who can install/enable https://github.com/marketplace/pre-commit-ci |
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 both!
As Ruff claims full compatibility with Black, it makes sense just to use one tool for both to prevent eventual collisions; also bumping Ruff to the latest
0.2.2
cc: @hugovk