[cleanup] Lint the project, add pre-commit hooks

[pukkandan]

Please follow the guide below

• You will be asked some questions, please read them carefully and answer honestly
• Put an x into all the boxes [ ] relevant to your pull request (like that [x])
• Use Preview tab to see how your pull request will actually look like

---

Before submitting a pull request make sure you have:

• Searched the bugtracker for similar pull requests
• Read adding new extractor tutorial N/A
• Read youtube-dl coding conventions and adjusted the code to meet them
• Covered the code with tests (note that PRs without tests will be REJECTED) N/A
• Checked the code with flake8

In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:

• I am the original author of this code and I am willing to release it under Unlicense
• I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

• Bug fix
• Improvement
• New extractor
• New feature

---

Description of your pull request and other information

• Lints the whole project with autopep8 and isort
• Add pre-commit config and CI workflow to enforce the style for future commits

Followed from #31327 (comment)

If you plan to squash this during merge, I would recommend making atleast 2 commits, one with 9201e8a, a90ebc0, 3abaaef and another with the rest

TODO

The POC reveals (7dc2e7b) what appears to be an actual bug in format_bytes() (str -> compat_str or compat_basestring).

• I don't think we should mix style changes with actual code change. I suggest patching this separately

Also for the isort commit (cafba86) there is a git blame problem.

• Should I add the commit(s) to a new git-blame-ignore-revs file? Depending on your merge workflow, the commit ids may change - so I want to confirm first. Or, you could always add it in a new commit after merging this

• Should changes be made to the contribution guide and PR template? In other words, do we want to instruct devs to use pre-commit, or is it sufficient to rely on the CI?

• If you wish for the changes to be applied automatically on PRs, you can revert the workflow and use https://pre-commit.ci instead. There are ways to do this with GH workflow too, but pre-commit.ci is a better solution

• When creating a release workflow (e.g. Add build workflow #30644), I suggest adding a step with pre-commit autoupdate to it

• The CI is expected to fail when there are new changes in master. Merging and committing with a pre-commit run --all-files will fix it. I'm not doing it rn since it'll have to be re-done after each commit to the master. Instead it makes sense to do it just once, before actually merging this

[dirkf]

Dealing with the easy one first, agreed. float_or_none() is the (separate) answer to that and the whole format_bytes() implementation from yt-dlp might as well be ported with format_decimal_suffix() which does use float_or_none(). I'll put that on the to-do list.

[pukkandan]

@dirkf Assuming you have no specific opinions on the TODOs posted, this PR should be fine as-is. Would it be possible to get this merged soon. (I have a similar patch ready for yt-dlp, but would prefer to merge it only after this one)

[dirkf]

Sorry, I've been lax in coming to views on your points, but am now a bit more up to speed.

If you plan to squash this during merge, I would recommend making at least 2 commits, one with 9201e8a, a90ebc0, 3abaaef and another with the rest

+1

Should I add the commit(s) to a new git-blame-ignore-revs file? Depending on your merge workflow, the commit ids may change - so I want to confirm first. Or, you could always add it in a new commit after merging this

No, doing it afterwards sounds good.

Should changes be made to the contribution guide and PR template? In other words, do we want to instruct devs to use pre-commit, or is it sufficient to rely on the CI?

If, as I infer from the next point, the style enforcement wouldn't automatically apply to a PR until merging, I'm inclined to say that we should just take that route instead (ie do whatever is necessary to apply the rules to commits in PRs). But if enforcement applies globally when a commit is made in a PR after the PR has been created, that should be enough.

Either way, it would be good to warn potential devs as to what will happen to their precious code, and perhaps that they can avoid surprises by configuring their forks in the same way.

If you wish for the changes to be applied automatically on PRs, you can revert the workflow and use https://pre-commit.ci instead. There are ways to do this with GH workflow too, but pre-commit.ci is a better solution

Well, actually this sounds good: I guess it's between that and the https://pre-commit.ci/lite.html version?

When creating a release workflow (e.g. Add build workflow #30644), I suggest adding a step with pre-commit autoupdate to it

If the effect is possibly to update some of the checks that are being applied, shouldn't it be a separate step before triggering the release?

[pukkandan]

If, as I infer from the next point, the style enforcement wouldn't automatically apply to a PR until merging, I'm inclined to say that we should just take that route instead (ie do whatever is necessary to apply the rules to commits in PRs). But if enforcement applies globally when a commit is made in a PR after the PR has been created, that should be enough.

If using current implementation:

• It will not run on existing PRs unless any new commit is pushed to it. Even a "merge with master" or a blank commit is sufficient.
• If the PR does not conform, the linter fails - with the action details explaining why. This is similar to the current flake8 action. We do not automatically push the changes, the dev must do so manually

If using pre-commit.ci

• Same as above, any new commit should trigger the CI. I don't know if it has some way to run without a new push
• If the PR does not conform, corrections are automatically pushed to the branch without user intervention as long as the dev has not disabled "Allow edits ... by maintainers"

Either way, it would be good to warn potential devs as to what will happen to their precious code, and perhaps that they can avoid surprises by configuring their forks in the same way.

Alright. I'll add something to docs

There are ways to do this with GH workflow too, but pre-commit.ci is a better solution

I guess it's between that and the pre-commit.ci/lite.html version?

I was not aware of this and was thinking we'd have to build the necessary workflow from scratch 😅. But it looks good

When creating a release workflow (e.g. Add build workflow #30644), I suggest adding a step with pre-commit autoupdate to it

If the effect is possibly to update some of the checks that are being applied, shouldn't it be a separate step before triggering the release?

Fair. My thought was that adding to build workflow would be easiest and predictable. But we could also add it as a new timed workflow (per week?). Or, if the automatic push to master is a concern, we could also let it create a PR, like dependabot.

Ofc, this is not relevant if using precommit.ci, since they already have a dependabot-like mechanism built in