Skip to content
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

Considering Ruff over Black #11734

Closed
wants to merge 5 commits into from
Closed

Considering Ruff over Black #11734

wants to merge 5 commits into from

Conversation

Avasam
Copy link
Sponsor Collaborator

@Avasam Avasam commented Apr 8, 2024

This PR explores using Ruff format in place of Black.

The main advantages are:

  • speed (there is a small, but noticeable difference when running pre-commit on the entire repo, for example)
  • Simplified configs and docs (only because we already use Ruff)
  • 1 less dependency
  • Arguably better formatting in some areas (especially where a comment previously prevented a line-wrap)

Eventually scripts could also be simplified as Ruff would be run using a single command astral-sh/ruff#8232

Disadvantages:

Comment on lines +19 to +24
exclude = [
# Exclude protobuf files because they have long line lengths
# Ideally, we could configure Ruff to allow longer line lengths
# for just these files, by adding a ruff.toml config file at their root.
"**_pb2.pyi",
]
Copy link
Sponsor Collaborator Author

Choose a reason for hiding this comment

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

Ruff is so fast and fixers are already run on these files as part of the generator scripts, I feel like this isn't even necessary.
I just kept if for now to have an apples-to-apples comparison.

@zanieb
Copy link

zanieb commented Apr 8, 2024

Exciting to see this! Note we added the formatter to our versioning policy and it should be quite stable!

This comment has been minimized.

@JelleZijlstra
Copy link
Member

I don't see much advantage in this change, though as a maintainer of Black I might be biased. I don't think formatting speed is a bottleneck for us.

This comment has been minimized.

@srittau
Copy link
Collaborator

srittau commented Apr 8, 2024

For the moment, I also prefer to keep black for formatting.

  1. Slightly different formatting introduces unnecessary churn.
  2. black is a PSF project.
  3. Due to black's maintainer structure, it might arguably be easier for typeshed to influence stub formatting decisions in black than in ruff.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Apr 8, 2024

Having to manually move type comments around isn't a good sign. Black basically just totally leaves those alone. Also note that ruff formatter currently relies on Black for its style. Black has historically collaborated with typeshed to determine Black's pyi style, so there's sort of a healthy collaboration / feedback loop there. I don't think there's much advantage here.

Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@Avasam
Copy link
Sponsor Collaborator Author

Avasam commented Apr 14, 2024

Thanks everyone for your feedback and additional context. I had an inkling about a sentiment towards keeping black, but I couldn't be sure without trying or asking.

I'll close this PR, but if you have any other opinion you'd like to share, feel free to do so here.

@Avasam Avasam closed this Apr 14, 2024
@Avasam Avasam changed the title From Black to Ruff Considering Ruff over Black Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants