Skip to content

chore: ruff format #6260

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

Merged
merged 3 commits into from
Oct 22, 2024
Merged

Conversation

bastimeyer
Copy link
Member

@bastimeyer bastimeyer commented Oct 21, 2024

This reformats the entire codebase using ruff format, which is almost identical to black's formatting:
https://docs.astral.sh/ruff/formatter/

For now, I've split the reformat changes into multiple commits, but I will squash them before merging, so the one giant commit can be added to .git-blame-ignore-revs afterwards. Having multiple commits is easier to review, because the diff shortstat is currently this: 257 files changed, 9880 insertions(+), 7909 deletions(-)

Instead of just running ruff format, the reformatting was done by using PyCharm with ruff's lsp set up, so there's probably some additional PyCharm reformatting logic included, not sure, but doesn't matter.

I've also had to skip the formatting in lots of places (# fmt: skip annotation after a whole statement), because tooling is usually not smart and the formatting often was chosen deliberately in order to highlight certain circumstances or to make it more legible.

Most of the diff is only indentation related though. For example, almost all of the pytest parametrization didn't align its parameters according to the expected style, in order to prevent one indentation level. That's all changed and trivial to review.

Another huge part of the diff are plugins of course. I tried to keep a common style of the pluginmatchers and their non-verbose regexes, but the annoying things here are HTTP requests with validation schemas, which almost all of them required a reformatting due to the parameter style of function calls with multi-line parameter values. Most of that is of course also only indentation, with new trailing commas added.

Apart from that, I believe I also changed/rearranged a few minor things here and there. Can't remember.


This PR also adds ruff format --diff to the linting CI workflow, so using the correct coding style is now mandatory. The docs were updated in this regard.

One thing that needs to be mentioned though is that there are two linting rules which we use which are incompatible with the formatter's auto-fix logic, namely

This doesn't affect its check logic (--check / --diff) and triggers errors in the linter, which is therefore fine.

Another thing worth mentioning is that the formatter uses ruff's "preview" rules. Those will only be changed in their minor releases though. And since we're pinning the ruff version, there's nothing to worry anyway.


I will go through the diff tomorrow again, in case I made some mistakes, and then merge.

@bastimeyer
Copy link
Member Author

Btw, GitHub's diff view is terrible for these kinds of changes where mainly whitespace was changed. Other tools have much better diff highlighting.

@gravyboat
Copy link
Member

I'm about 50% through reviewing this locally. I'll try to get the rest done tonight or tomorrow.

Copy link
Member

@gravyboat gravyboat left a comment

Choose a reason for hiding this comment

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

Alright I'm going to mark this as approved. As you noted please do your secondary check just in case I also missed something. I don't see anything problematic currently however. Nice job.

@bastimeyer
Copy link
Member Author

Thanks for taking the time having a second pair of eyes on the changes...
I force pushed a few more cleanups and squashed.

Changes should be fine now (for the most part). Anything else can be changed/fixed later.

Going to merge now.

@bastimeyer bastimeyer changed the title chore: reformat everything using ruff format chore: ruff format Oct 22, 2024
@bastimeyer bastimeyer merged commit ac3a308 into streamlink:master Oct 22, 2024
23 checks passed
@bastimeyer bastimeyer deleted the tools/ruff/format branch October 22, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants