-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
Convert tests
to ruff-format
#21129
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: main
Are you sure you want to change the base?
Convert tests
to ruff-format
#21129
Conversation
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
This pull request has merge conflicts that must be resolved before it can be |
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.
Code Review
This pull request successfully converts the tests
directory to use ruff-format
. The changes are well-contained and achieve the stated goal.
Here's a summary of my review:
- Configuration: The updates to
.pre-commit-config.yaml
andpyproject.toml
correctly switch the formatting responsibility for thetests
directory fromyapf
andisort
toruff-format
. The use of a verbose regex in.pre-commit-config.yaml
is a nice improvement for readability. - Code Formatting: The changes across all Python files in the
tests
directory are purely stylistic and consistent with the application of a new code formatter. I've reviewed the diffs and found no unintended logical changes. - Correctness: The changes appear correct and align with the RFC. The CI test runs will be the ultimate verification that no regressions were introduced.
Overall, this is a solid refactoring effort that improves tooling and consistency. I have no high
or critical
issues to report. The PR looks good to merge.
cf4d3a8
to
9bd0dea
Compare
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
9bd0dea
to
ceb7cc2
Compare
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
This pull request has merge conflicts that must be resolved before it can be |
RFC: #17657
If we are converting directory
x
:x
, we add a localpyproject.toml
which:ruff
's line length to 88 (it's own default)isort
in ruffx
to the list of files to runruff-format
on in.pre-commit-config.yaml
x
to the list of ignores in theyapf
andisort
config in the rootpyproject.toml