-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-132390: Apply Ruff linting to Tools/build
#132391
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
gh-132390: Apply Ruff linting to Tools/build
#132391
Conversation
Tools/build
with existing ruff
configuration
@AA-Turner @hugovk Does it look acceptable or would you prefer to just exclude the files instead of ruffing them? |
This reverts commit af4a0ea.
I'll actually hold the merge a bit more so that code owners can also give their opinion. |
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.
Looks like a nice cleanup to me, but I'm not the person maintaining most of these scripts :-)
make sure to get signoff from the relevant maintainers before landing this. Not everybody on the team is necessarily on board with stricter linting in CI!
Yes, that's why I eventually decided to hold off the merge! |
Ok so here is how I decided to make the change:
|
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 for generate_sbom.py
.
@@ -455,7 +453,7 @@ def do_unixy_check(manifest, args): | |||
okay &= _report_unexpected_items( | |||
extra_defs, | |||
'Some extra declarations were found in "Include/Python.h" ' | |||
+ 'with Py_LIMITED_API:') | |||
'with Py_LIMITED_API:') |
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.
Here, I did want to point out that it's a single string and not a forgotten comma.
Could you point me to the reasoning behind this change?
Tools/build
using existing ruff configuration #132390