Skip to content

GH-139590: Run ruff format on pre-commit for Tools/wasm #139591

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Oct 8, 2025
Merged

GH-139590: Run ruff format on pre-commit for Tools/wasm #139591

merged 11 commits into from
Oct 8, 2025

Conversation

savannahostrowski
Copy link
Member

@savannahostrowski savannahostrowski commented Oct 5, 2025 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request Oct 5, 2025
Closed
hugovk
hugovk approved these changes Oct 5, 2025
View reviewed changes
sobolevn
sobolevn reviewed Oct 5, 2025
View reviewed changes
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question / confusion: we have both black and ruff format inside the repo 🤔

How do we choose when to use which? Maybe we can migrate to a single tool (ruff) in the future?

@savannahostrowski
Copy link
Member Author

savannahostrowski commented Oct 5, 2025 •
edited
Loading

@sobolevn This is a good question. Since the JIT code and now WASI are the only dirs using black, it may be worth moving everything over to Ruff.

@brettcannon Was there a particular reason that you were looking to use black over Ruff (listed in your TODO list on your website 😄 )?

For the JIT, I don't think it really matters. I'd previously added the configuration to run black over JIT code in #118537, but I'm happy to move over to Ruff if that's what we've decided on.

@AlexWaygood
Copy link
Member

question / confusion: we have both black and ruff format inside the repo 🤔

How do we choose when to use which? Maybe we can migrate to a single tool (ruff) in the future?

See #133123 for previous discussion

@AA-Turner
Copy link
Member

AA-Turner commented Oct 5, 2025 •
edited
Loading

I'd also support using ruff format (for speed if nothing else), switching Tools/jit/ to ruff is awaiting their team making some improvements to match/case formatting, which I don't believe applies here.

a

@savannahostrowski
Copy link
Member Author

I missed that thread about the JIT using Ruff as I was out getting married that week 😅. I appreciate the context. I'll wait for Brett's input, but it seems like moving things over to Ruff going forward/as soon as feasible, would be the right move.

emmatyping
emmatyping reviewed Oct 5, 2025
View reviewed changes
emmatyping
emmatyping reviewed Oct 5, 2025
View reviewed changes
savannahostrowski and others added 3 commits October 5, 2025 16:29
@savannahostrowski @hugovk
Update Tools/wasm/wasi/__main__.py
e8f45e5 
Co-authored-by: Hugo van Kemenade <[email protected]>
@savannahostrowski @hugovk
Update Tools/wasm/wasi/__main__.py
bb35fd0 
Co-authored-by: Hugo van Kemenade <[email protected]>
@savannahostrowski @hugovk
Update Tools/wasm/wasi/__main__.py
d8dab97 
Co-authored-by: Hugo van Kemenade <[email protected]>
@hugovk
Copy link
Member

hugovk commented Oct 6, 2025

A benefit of Ruff is it fixes the quote things Emma and I suggested.

Alternatively, we can add the ISC (flake8-implicit-str-concat) rule, like in Tools/build/.ruff.toml.

@brettcannon
Copy link
Member

Was there a particular reason that you were looking to use black over Ruff (listed in your TODO list on your website 😄 )?

Nope, just that I don't think ruff format was being used anywhere while Black was. I personally don't care which one gets used.

@savannahostrowski
Copy link
Member Author

Sounds good - will update to use Ruff later today!

@savannahostrowski savannahostrowski changed the title GH-139590: Run black on pre-commit for WASI GH-139590: Run ruff format on pre-commit for WASI Oct 7, 2025
@savannahostrowski
Copy link
Member Author

Not sure if we need to set a custom .ruff.toml for this directory or if we're happy with the project configuration. Looks pretty solid to me?

@brettcannon
Copy link
Member

Should this get backported to make backporting other things easy? Or maybe just the reformatting?

@hugovk
Copy link
Member

hugovk commented Oct 7, 2025

Yes, let's backport, if it's not too tricky.

I think that the default config does not select many useful plugins. I propose adding Tools/wasm/.ruff.toml file with contents similar to Tools/build/.ruff.toml. I would also include Tools/wasm/wasi.py (it is deprecated, but still)

Later this file can also be used to format Tools/wasm/emscripten as well.

For a followup:

It's worth checking and moving common rules from subdir .ruff.toml files into the root one.

There might be some rules that are in most subdir configs but not all, and it could be worth moving them up so those rules do apply for all.

@savannahostrowski savannahostrowski changed the title GH-139590: Run ruff format on pre-commit for WASI GH-139590: Run ruff format on pre-commit for Tools/wasm Oct 7, 2025
@savannahostrowski
Copy link
Member Author

savannahostrowski commented Oct 7, 2025 •
edited
Loading

I made the pre-commit rule check for all of Tools/wasm since it seemed odd to me that we wouldn't handle emscripten/ at the same time (i.e., the only unchecked subdir), but let me know if anyone vehemently disagrees.

freakboy3742
freakboy3742 approved these changes Oct 7, 2025
View reviewed changes
Copy link
Contributor

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 No objection to turning on Ruff for the Emscripten paths as well.

@savannahostrowski savannahostrowski added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Oct 8, 2025
@savannahostrowski savannahostrowski enabled auto-merge (squash) October 8, 2025 02:04
@savannahostrowski savannahostrowski merged commit a15aeec into python:main Oct 8, 2025
43 checks passed
@miss-islington-app
Copy link

Thanks @savannahostrowski for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 8, 2025
@savannahostrowski @hugovk @miss-islington
pythonGH-139590: Run ruff format on pre-commit for Tools/wasm (pyth…
e6a1d23 
…onGH-139591)

(cherry picked from commit a15aeec)

Co-authored-by: Savannah Ostrowski <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
@miss-islington-app
Copy link

Sorry, @savannahostrowski, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker a15aeec29efa5b3d5d5568278c13bb3fc45f52ef 3.13

@bedevere-app
Copy link

bedevere-app bot commented Oct 8, 2025

GH-139744 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Oct 8, 2025
savannahostrowski added a commit to savannahostrowski/cpython that referenced this pull request Oct 8, 2025
@savannahostrowski @hugovk
pythonGH-139590: Run ruff format on pre-commit for Tools/wasm (pyth…
13ac19d 
…on#139591)

Co-authored-by: Hugo van Kemenade <[email protected]>
(cherry picked from commit a15aeec)
@bedevere-app
Copy link

bedevere-app bot commented Oct 8, 2025

GH-139745 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Oct 8, 2025
@bedevere-app
Copy link

bedevere-app bot commented Oct 8, 2025

GH-139745 is a backport of this pull request to the 3.13 branch.

@sobolevn sobolevn mentioned this pull request Oct 8, 2025
Merged
savannahostrowski added a commit that referenced this pull request Oct 8, 2025
@savannahostrowski @hugovk
[3.13] GH-139590: Run ruff format on pre-commit for Tools/wasm (GH-…
5a98f85 
…139591) (#139745)

Co-authored-by: Hugo van Kemenade <[email protected]>
savannahostrowski added a commit that referenced this pull request Oct 8, 2025
@miss-islington @savannahostrowski @hugovk
[3.14] GH-139590: Run ruff format on pre-commit for Tools/wasm (GH-…
c5ab1b7 
…139591) (#139744)

Co-authored-by: Savannah Ostrowski <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sobolevn sobolevn sobolevn left review comments

@freakboy3742 freakboy3742 freakboy3742 approved these changes

@brettcannon brettcannon brettcannon approved these changes

@hugovk hugovk hugovk approved these changes

@emmatyping emmatyping emmatyping approved these changes

@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood AlexWaygood is a code owner

@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner

Assignees

@savannahostrowski savannahostrowski

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@savannahostrowski @AlexWaygood @AA-Turner @hugovk @brettcannon @freakboy3742 @sobolevn @emmatyping