Precommit CI Integration

[C-Loftus]

Currently the repo is just using flake8. pre-commit is a way to run the CI suite in a way that is a bit easier to maintain, and easier to run locally. This should make the git diffs in PRs cleaner if people are using different auto formatters since it autofixes and then recommits to have a standard formatter. (ruff for Python, prettier for JS/HTML)

We can still keep flake8 for the time being since ruff covers almost everything with the exception of a few flake8 rules.

You can run all of these hooks locally by running pre-commit after installing with pip

[C-Loftus]

To actually implement this, you need to enable the one click GitHub repo integration via https://pre-commit.ci/ and then it just runs automatically on a pull request.

The only downside for pre-commit would be the fact that we would have to do this for every repository we want pre-commit for (hence a monorepo is sometimes more natural to work with but it would certainly still work in our situation).

If you want to see how it looks in a real deployment, you can take a look at this PR: C-Loftus/sight-free-talon#29

[webb-ben]

Am I correct in saying adding it here will at least enable downstream forks to be able to have their pre-commit checks run on PR by registering it on https://pre-commit.ci/?

Can we enable this org wide like you can with Travis CI?

Would we need to also populate .git/hooks/* in the "template" or would that be an additional manual step to enable the pr checks?

An unfortunate reality but not the biggest lifts if we are just managing this by project. @czawlytko @mmatkinson7 thoughts?

[C-Loftus]

Would we need to also populate .git/hooks/* in the "template" or would that be an additional manual step to enable the PR checks?

pre-commit installs all the git hooks automatically. So we do not touch the .git directory. Any of the work around managing git hooks is abstracted away.

Am I correct in saying adding it here will at least enable downstream forks to be able to have their pre-commit checks run on PR by registering it on https://pre-commit.ci/?

Correct, so essentially any repository that is using a pre-commit config file will have the config be sourced by the integration via the website and then the pre-commit bot will run the hooks (which is free obviously as well I should mention just to be totally clear).

Can we enable this org wide like you can with Travis CI?

I don't believe so. Could be wrong in this but I haven't seen it.

[czawlytko]

This sounds good to me. If @mmatkinson7 is on board lets do it. Maybe @C-Loftus can walk us through how to set this up correctly for existing repos?

[C-Loftus]

This sounds good to me. If @mmatkinson7 is on board lets do it. Maybe @C-Loftus can walk us through how to set this up correctly for existing repos?

Yes for sure. I personally don't have permission in the repo to integrate it, but certainly happy to walk through it with anyone.

And before we merge anything in the template repo, we can do a PR against https://github.com/internetofwater/register.geoconnex.us for a more simple test run on a smaller codebase to see if it is satisfactory. (Since template repo changes don't easily propagate downstream we will have to do that anyways at some point)

[mmatkinson7]

I'm super into this but I definitely don't know enough to know how to implement it in my repo's that have already been created. I am down to have this as part of the template starting now and have @C-Loftus walk us through what we need to change and what changes in our routines.

[C-Loftus]

This config seems to stable and working with the register.geoconnex.us repo now. I have it so it includes both ruff as a formatter and prettier for JS/HTML. Some repos obviously do not need both, however, I think having both in the template makes it a bit easier to manage (rather than having less and needing to add more via post hoc commits from the template)

[C-Loftus]

Added nbstripout as a pre-commit hook. Note that

When installed as a pre-commit hook and running locally as a local git hook by doing pre-commit install, nbstripout will modify your working copy!**

Thus it is safer to not run pre-commit install locally and always have it run on the remote via the github integration. Should be all good, never run into issues personally, but I just don't want to this to ever run into a situation where there is a unexpected upstream bug and work is lost locally after committing.

So to summarize, don't think there is anything wrong with this, but it is always 100% safest to run it such that it is updating the remote branch

[C-Loftus]

Not a big deal, but it appears that someone enabled pre-commit for every repo in the organization. I reverted this assuming that's ok. Feel free to ping me and I am happy to add any other repos. Should be easy for anyone in the org though as well.

For the time being we will just want to manually add pre-commit integrations for the repos we want since not all repo's have the config file.
If you run the integration within repos without the pre-commit-config.yml in the root, you will get CI failures.

See cgs-earth/sensorthings-action#1 as an example