style: add pre-commit and new checks

[henryiii]

Adds pre-commit as a check-driver (in turn, driven by Nox), and adds some new checks - I added most of my favorites, figuring we could scale back or split up as needed.

• Black (like before)
• Pre-commit-hooks: some common simple checks, like for nice end-of-files, etc.
• PyUpgrade, set to 3.6+: A fantastic tool that can keep syntax fresh and clean.
• isort (like before)
• setup-cfg-fmt: Some standardisation for setup.cfg, also keeps trove classifiers accurate
• pycln: Cleans out unused imports automatically (flake8's check but mutating)
• yesqa: cleans out noqa's that are no longer needed
• flake8: This time, with bugbear, which looks for problematic expressions that might be correct but tend to confuse readers or cause bugs
• mypy (as before)
• codespell: Looks for common misspellings. Very handy!
• pygrephooks: Avoid allowing blanket noqa or type ignores, type commends, and some other common mistakes.
• shellcheck: looks for mistakes in shell scripts. Might not be needed, not sure if there are any shell scripts.

Removes "nox -s blacken", since some checks modify in place anyway, and everything is in git.

I'll comment on the changes a bit inline.

[FollowTheProcess]

I'm on board with this and agree with the changes overall, there are a lot of changes in this PR but they all seem to be small linty/style type stuff which is fine!

However since it's a reasonable change to how our CI works (albeit not to the user, they simply run nox as normal which is great!) I'd like to get some other maintainers thoughts/comments before merging 🙂

[cjolowicz]

LGTM

Are you planning to move this check to pre-commit.ci in a separate PR? Otherwise, we should probably switch to the pre-commit action for the lint job. This speeds up CI by caching the pre-commit environments.

Edit: Actually, the lint step is surprisingly fast even without caching.

[FollowTheProcess]

Happy to merge as is or do we want to change to the pre-commit action?

[henryiii]

No, that action is deprecated in favor of pre-commit.Ci. Someone with permissions will have to enable pre-commit.ci for the repo.

[FollowTheProcess]

No, that action is deprecated in favor of pre-commit.Ci. Someone with permissions will have to enable pre-commit.ci for the repo.

Okay, sounds like that's best done under a new PR then

[DiddiLeija]

I don't have experience with pre-commit, but it seems like it will fit well with Nox!

Also, and sorry for my ignorance, but... will pre-commit do the changes for us, or it will just fail?

[FollowTheProcess]

I don't have experience with pre-commit, butit seems like it will fit well with Nox!

Also, and sorry for my ignorance, but... will pre-commit do the changes for us, or it will just fail?

It will just fail. It effectively just runs all the linters specified in the config file. You can use it how we do here (by calling pre-commit run either manually or from another tool like Nox) or you can install it as a git pre-commit hook so that every time you commit it will run and abort the commit if it fails.

Some people (myself included) don't like the latter option as long running checks can interrupt your commit flow but it depends on what you have configured

[Tolker-KU]

I don't have experience with pre-commit, butit seems like it will fit well with Nox!

Also, and sorry for my ignorance, but... will pre-commit do the changes for us, or it will just fail?

It will just fail. It effectively just runs all the linters specified in the config file. You can use it how we do here (by calling pre-commit run either manually or from another tool like Nox) or you can install it as a git pre-commit hook so that every time you commit it will run and abort the commit if it fails.

Some people (myself included) don't like the latter option as long running checks can interrupt your commit flow but it depends on what you have configured

Does that imply that this PR leaves no easy way for a developer to run black and isort to make changes as the "blacken" session does now?

[henryiii]

I almost always run pre-commit run -a, but pre-commit install is quite speedy, as the git hook only looks at changed files. It even handles partial stages. And git hooks can be skipped with -n in a pinch. I just like having control.

Pre-commit supports both changing checks (black, pyupgrade) as well as failing checks (flake8). The changing ones are better, since they can be fixed automatically by pre-commit.ci and pushed.

[DiddiLeija]

Does that imply that this PR leaves no easy way for a developer to run black and isort to make changes as the "blacken" session does now?

Well... not exactly. As far as I know, if you install pre-commit, you can run it by yourself, and it should fix your mistakes? (EDIT: @henryiii already posted about this)

BTW, I would suggest to add --show-diff to the pre-commit command called by Nox, to show the Git diff where the failure is?

[FollowTheProcess]

Does that imply that this PR leaves no easy way for a developer to run black and isort to make changes as the "blacken" session does now?

Sorry just re-read my reply and it wasn't too clear! As @henryiii said it can do both changing and non-changing fixes, my saying it will fail was confused by pre-commits commit hook functionality which would fail the commit even if it managed to change the code e.g. black, pyupgrade etc as you'd have to re-commit the changes.

Looks like with pre-commit run this isn't an issue :)

[cjolowicz]

Thank you @henryiii