-
Notifications
You must be signed in to change notification settings - Fork 546
Format Python and Bash scripts using ruff and shfmt #1454
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
base: master
Are you sure you want to change the base?
Conversation
Moved
The |
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've been meaning to add Ruff format to the C repo for a while, so this is good precedent. LGTM
etc/format.py
Outdated
return res.returncode == 0 | ||
|
||
|
||
def _get_files_matching(pat: str) -> Sequence[Path]: |
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 function is not used (it's not used in the C repo either, I just haven't gotten around to deleting it yet)
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.
Posted mongodb/mongo-c-driver#2102.
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.
LGTM
Extends automated formatting to Python and Bash scripts using the existing
uv
project file.Important
This PR only selects and provides the tools required for automated formatting of Python and Bash scripts. However, it does not check or enforce that these files are correctly formatted. The
lint
task is untouched except to use the newetc/format.py
script. Theetc/format.py
itself script remains specific toclang-format
only.The new
etc/ruff-format-all.sh
andetc/shfmt-format-all.sh
scripts are included for convenience and reference ("how to use these tools"). They do not need to be used directly (e.g. directly runninguv run ruff format
is just fine). I expect/hope these will be integrated intoetc/format.py
(similar to how it replaces theetc/clang-format-all.sh
script) in the near future.Similar changes are planned for the C Driver shortly following this PR.
For C and C++ source files,
tools/format.py
from mongodb/mongo-c-driver#2050 is imported and reused here asetc/format.py
, with minor tweaks to support the C++ Driver directory structure. This replaces theetc/clang-format-all.sh
script. Relevant documentation and references have been updated accordingly.Note
Adding support for
uv run format
, similar touv run mc-evg-generate
, was considered but postponed for now.For Python files, this PR proposes using Ruff. For consistency with our updated ClangFormat preferences, the column width is set to 120. For consistency with recent Python code,
quote-style = "single"
andindent-style = "space"
are also set. Note multiline strings unconditionally use double-quotes despitequote-style = "single"
for conformance with PEP 8 and PEP 257.The expected commands to format all Python files is documented and supported by
etc/ruff-format-all.sh
. Note theruff check --fix
command required to sort and groupimport
statements (see: astral-sh/ruff#8232), first by "standard library", then "third party", then "first party" (see: Ruff FAQ). I don't believe this ordering is customizeable (see: astral-sh/ruff#7615).Unfortunately I don't think there's anything we can do about the "two-space lead for trailing comments" formatting either (e.g.
print("example") # comment
). There is a massive semi-related unresolved issue astral-sh/ruff#7684 concerning formatting of comments. The default behavior is left as-is for now.Formatting of
pyproject.toml
itself (as a TOML file) is also deferred, as there does not seem to be an immediately-obvious community-supported tool to integrate and use for this purpose at this time (see: toml-lang/toml#532). Wait and hope for astral-sh/ruff#10738 to be implemented soon...?Note
Ruff also supports linting Python scripts via
ruff check
. This is not integrated into any scripts at this time. However, as a drive-by improvement, all currently-outstanding warnings emitted byruff check
are fixed by this PR using both--fix
and manual edits. Addingpyright
was considered but postponed to keep this PR focused on formatting only.For Bash scripts, this PR proposes using shfmt. Unlike Ruff, there is no convenient auto-detection of relevant project files, so
etc/shfmt-format-all.sh
uses a simplefind
command to find and filter.sh
scripts under.evergreen
,etc
, andexamples
. For consistency with recent Bash code, indentation is set to two spaces with-i 2
. Althoughshfmt
also supports--simplify
, but this flag is omitted for now. This flag can be included if preferable.Note
Linting Bash scripts can be done using
shellcheck
via theshellcheck-py
package, but this too is postponed to keep this PR focused on formatting only. However, this can be done manually usinguv run --with shellcheck-py shellcheck -x <files>
.