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

chore: switch to ruff format over black #765

Merged
merged 7 commits into from
Jan 24, 2025
Merged

Conversation

willmurphyscode
Copy link
Contributor

We've had both black and ruff making changes to the code on lint/pre-commit for a long time.

Ruff now claim that their formatter is a drop-in replacement for black.

This PR switches make format from using black to using ruff format.

The rest of the diff is the result of running make format on the code after the switch, which look like only line length changes.

When we had black and ruff, both had a line-length setting, and black was set to 130 but to ignore some files, and ruff was set to 150 but to ignore no files. In ruff format, there's no way to set the line length to be different for linting and formatting, so some lines are changing from 130 to 150. So there used to be some files with a 130 line length limit and some files with a 150 line length limit. Now all files are at 150.

We could also standardize on 130. I'll defer to whatever the team wants here.

Also, black used to allow (require?) a blank line between class Foo: and __some_var__ = whatever. ruff format by default takes this line out. I haven't figured out a way to tell ruff format to allow this, so these blank lines are going away.

The total speed up benefit of this is not as much as I'd hoped - make static-analysis is pretty variable in how long it takes, but it seems between 200 and 500 millis faster on this branch, out of a total of about 4000 millis, so about maybe 10% faster). Still, this gets run a lot, so a small speed-up is welcome.

I think the real benefit is having only one tool that care about line length. Having a list of strings as exclude paths instead of a multi line regex is nice too.

@willmurphyscode willmurphyscode added the run-pr-quality-gate Triggers running of quality gate on PRs label Jan 17, 2025
@willmurphyscode willmurphyscode marked this pull request as ready for review January 20, 2025 15:17
@@ -57,8 +57,8 @@ repos:
hooks:

# note: this is used in lieu of autopep8 and yapf
- id: black
name: black
- id: format
Copy link
Contributor

Choose a reason for hiding this comment

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

Good change to generalize this rather than use the tool name

@@ -77,7 +76,7 @@ types-requests = "^2.28.11.7"
mypy = "^1.1"
radon = ">=5.1,<7.0"
dunamai = "^1.15.0"
ruff = ">=0.5.1,<0.9.2"
ruff = "^0.9.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

confirmed v0.9.2 is latest from 5 days ago

change_start_date: (
str | datetime.datetime | None
) = None, # note: if you specify a changeStartDate, you must also specify a changeEndDate
results_per_page: (int | None) = None, # from api docs: "it is recommended that users of the CVE API use the default resultsPerPage value"
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks much better

@willmurphyscode willmurphyscode merged commit d90e938 into main Jan 24, 2025
21 checks passed
@willmurphyscode willmurphyscode deleted the chore-ruff-format branch January 24, 2025 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-pr-quality-gate Triggers running of quality gate on PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants