Replacing isort with Ruff #10912
Replacing isort with Ruff #10912
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| extra_standard_library = [ | ||
| "typing_extensions", | ||
| [tool.ruff.isort] | ||
| combine-as-imports = true |
There was a problem hiding this comment.
I recommend also split-on-trailing-comma = false, for closer isort-like behavior.
Otherwise Ruff doesn't collapse multiline imports (even if they do fit on one line).
There was a problem hiding this comment.
Regardless of intentions for the default value, I'm down to keep imports on a single line when possible anyway. Especially since we have skip_magic_trailing_comma = true configured for Black
Thanks for letting me know of this difference.
This comment has been minimized.
This comment has been minimized.
scripts/create_baseline_stubs.py
Outdated
| def run_ruff(stub_dir: str) -> None: | ||
| print(f"Running Ruff: ruff {stub_dir}") | ||
| subprocess.run([sys.executable, "-m", "ruff", stub_dir]) | ||
| subprocess.run([sys.executable, "-m", "ruff", stub_dir, "--fix-only"]) |
There was a problem hiding this comment.
Fix errors it can fix, and don't report on things it can't.
Ruff now recommends explicitly adding check:
| subprocess.run([sys.executable, "-m", "ruff", stub_dir, "--fix-only"]) | |
| subprocess.run([sys.executable, "-m", "ruff", "check", stub_dir, "--fix-only"]) |
There was a problem hiding this comment.
Ruff now recommends explicitly adding check
Right because of ruff format and to be explicit. I'll update other code and doc locations too.
scripts/generate_proto_stubs.sh
Outdated
| @@ -73,7 +73,7 @@ PROTO_FILES=$(grep "GenProto.*google" $PYTHON_PROTOBUF_DIR/python/setup.py | \ | |||
| # shellcheck disable=SC2086 | |||
| protoc_install/bin/protoc --proto_path="$PYTHON_PROTOBUF_DIR/src" --mypy_out="relax_strict_optional_primitives:$REPO_ROOT/stubs/protobuf" $PROTO_FILES | |||
|
|
|||
| isort "$REPO_ROOT/stubs/protobuf" | |||
| ruff "$REPO_ROOT/stubs/protobuf" --exit-non-zero-on-fix --fix-only | |||
There was a problem hiding this comment.
I think --exit-non-zero-on-fix is backwards. If fixing happens, we want ruff to exit with success so that the script continues. It uses set -e, so nonzero exit code would stop the script here.
There was a problem hiding this comment.
oops, you're right.
I'm also wondering whether --fix-only is wanted here. If you're gonna get errors later, may as well report them now? As long as it doesn't block script execution. Same with create_baseline_stubs
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
This PR explores using Ruff for linting and sorting imports.
This should lead to less configurations, one less dependency and faster imports auto-formatting
Ruff respects isort's action comments like
isort:skip_fileThere's some overlap with #10910 since both need to initially add the Ruff check in github actions.
Closes #10911