Add pre-commit configuration

[Guts]

In this PR

• add classical git hooks through pre-commit to prevent "simple" issues when pushing code
• add basic flake8 git hook, reusing configuration from setup.cfg and selecting most important flags
• add specific section for https://pre-commit.ci/ - recommended because it automates hook updates and quick fixes when it's possible. This requires that a member with write permissions registers the repository on the application.
• add required as extra dependencies into setup.py for development

Goal

A little step to improve code reliability.

Related issues

Related to #755

[Guts]

ping @jelmer and @ix5

[jelmer]

isn't this severely limiting what errors we'll get?

[Guts]

It's not to be too frustrating at the time of the commit or a PR.
If you prefer, we can enlarge this selection or apply everything.

[ix5]

I like your effort, @Guts. This certainly seems like something that could help catch little slip-ups in a fast-paced development environment.
I've left a few comments in the code.

In general though, I think running linting tools should be up to the discretion of individual committers. We don't want to force them to install yet another framework on their machine (with auto-downloaded hook repos from a to their eyes untrusted Github source).

I'm perfectly fine with recommending running flake8 before committing. flake8 is being run as CI on PRs already.

I'm also a bit confused because pre-commit seems to be two things at the same time:

• A framework for running "hooks" from its GitHub hook repository (which needs to be updated regularly) by creating a virtualenv (which I can't find any info about)
• A CI cloud service (free?) which requires push access for its bot to contributors' branches to push what it thinks are trivial fixes

The first one, as I've stated above, should in my opinion reside with individual contributors. We shouldn't force anyone to use it, but having the config for people who might want it seems a good idea.
The second one seems a bit intrusive. It's another unknown actor to trust. For my taste, the whole "sprinkle some packages on it" vibe of nodejs is not something I'd want to bring to this project. But then again, I'd not be entirely opposed to it. Waiting for other maintainers' opinions on this.

Don't get me wrong, I'm very much grateful for your efforts @Guts, but it seems this project will soon be 90% configs for some type of devops service while actual, structural or architectural issues are left hanging. If this helps us write unit tests or invites more contributors, that's a positive outcome. But as they say, adding fancy new things is easy, fixing bugs is hard ;)

[ix5]

Will this catch isso/foo/bar.py as well? I'm not familiar with the regexp(?) engine that interprets this, but will .* match subfolders (slashes) too?

[ix5]

We don't have any bash scripts afaik

[ix5]

Do we have YAML files apart from the ansible example?

[ix5]

Wouldn't we want tests formatted correctly as well?

[ix5]

Am I understanding this correctly that there'll be a little bot auto-committing to this repo? I'd rather avoid this. Proposing changes is fine, but commit rights seem a bit overkill.
Or will this only auto-commit what it thinks are fixes to the branches of PR authors which we can then squash?

[jelmer]

In general though, I think running linting tools should be up to the discretion of individual committers. We don't want to force them to install yet another framework on their machine (with auto-downloaded hook repos from a to their eyes untrusted Github source).

I'm perfectly fine with recommending running flake8 before committing. flake8 is being run as CI on PRs already.

Hmm, I'd forgotten about that. Adding flake8 to the "dev" dependencies seems reasonable. I'm not sure that the pre-commit config really adds much in that case.

-1 on automatic updates by third party bots from me as well.

Jelmer