Reinclude linters as pre-commit hooks

[fredjaya]

Already run as a GitHub Action, but mainly so you can run locally. Add to dev_install.

https://github.com/cogent3/c3dev/issues/23#issuecomment-1475678489

[GavinHuttley]

I've played with this and it's now clear my original objective cannot be solved easily, so I will discuss the purpose of this here and then suggest the next step.

We want all code in a PR to have been run through the correct versions of black and isort. This means when we review, we focus our comments on the code logic, not the contributor's adherence to our style requirements.

GitHub actions run by the cogent3 GitHub organisation cannot modify the contributor's repo. So we need the contributor to do it either on their machine or in their own GitHub account before making the PR.

So I think we can do two things here:

1. add a new GitHub action which is run (just on ubuntu, one version of python) before the test suite and fails if the black/isort commands fail
2. adapt the existing GitHub linter action so that it is triggered on the user's GitHub account on a push. Tell them how to set it up in our dev instructions. By using the versions of black / isort and OS as per our check, this should be the least burden on users.)

@khiron what do you think?

[khiron]

I definitely like the idea of running black/isort on just one version of python for a quick check for all pushes to the dev branch of the cogent3 repository. We can also just check-out changed code in that workflow, to make it extra quick. Also, linting before testing is a good call. Fail early is good.

pre-commit hooks in the local git repository should be the preferred strategy for devs who don't like getting their PR's bounced.

I recall developing a strategy for misnaming pre-commit hooks in the repository that the developer just had to rename locally to implement them (and a .gitignore strategy that didn't consider the renaming to be a committable change). That way, the code was part of the repo, but it only got executed locally if the developer made the required rename.

EDIT: Yup found it in the scratch repo I built for setting up scriv.

We add the following to .gitignore .github/local-hooks/pre-commit

and add the following 3 files to the /.github/ folder

https://github.com/khiron/testscriv/tree/master/.github/local-hooks

[just made this scratch repo public]

[GavinHuttley]

I agree the dev should introduce the precommit on their machine, but again thinking of the shortest path for slightly less sophisticated dev's, making it so their GitHub repo does it at least means they don't need to do more command line until they're ready.