Add pre-commit.ci

[b-rowan]

Adds proper pre-commit handling for PRs, along with pytest being run in a separate github action.

Closes #80

[tsagadar]

If I understand it correctly (no deep knowledge if github actions though)

• pytests would be executed for every PR on GitHub - but no longer as part of pre-commit on the dev machine
• all linting / formatting checks are only executed as part of pre-commit on the dev machine

Should we not have both activities be executed for every PR on GitHub - in case that the developer is not using pre-commit?

And why not keep pytest as part of pre-commit? The only downside I saw so far is that during a commit the server cannot run in parallel.

[b-rowan]

If I understand it correctly (no deep knowledge if github actions though)

• pytests would be executed for every PR on GitHub - but no longer as part of pre-commit on the dev machine

• all linting / formatting checks are only executed as part of pre-commit on the dev machine

Should we not have both activities be executed for every PR on GitHub - in case that the developer is not using pre-commit?

And why not keep pytest as part of pre-commit? The only downside I saw so far is that during a commit the server cannot run in parallel.

This is actually not correct. pytest is only being removed from the pre-commit.ci run, it still gets run locally. It just breaks when being run on the CI server, so I moved it to github actions.

Basically, everything gets executed on every PR push, and can be run locally if pre-commit is installed.

[b-rowan]

Here's a better breakdown of what's actually happening here -

pre-commit.ci - Remote CI server, runs pre-commit and can commit any changes needed (such as formatting fixes) automatically.

• Fails to run pytest, since that uses a locally configured hook.
• Because of this failure, it is disallowed from running the pytest hook. https://github.com/UpstreamDataInc/goosebit/blob/d8e6a30cd1cf28980cffa78a8be2f5494388e585/.pre-commit-config.yaml#L1-L3

GitHub Actions - Local action added to run pytest because it is not being run on pre-commit.ci https://github.com/UpstreamDataInc/goosebit/blob/d8e6a30cd1cf28980cffa78a8be2f5494388e585/.github/workflows/pr-run-tests.yml

pytest-md-report - Runs as part of the pytest action, it will display a chart of failed tests as a comment if the tests fail.

pre-commit - Locally run pre-commit config. No changes, this works exactly the same as before.

This means that all in all -

• Local pre-commit has not changed.
• pytest is run on PRs via an action, and will comment a formatted table with failures when tests fail.
• pre-commit hooks are run on PR with the exception of pytest, since it would break, and is handled by a separate action because of that. The pre-commit hooks may also make changes to the PR with a commit.

[tsagadar]

Is it possible that you did not commit pre-commit.ci?

[b-rowan]

Is it possible that you did not commit pre-commit.ci?

It's a website, technically installed as a github app.

https://pre-commit.ci

You can also take a look at the checks for this PR for more info.

[tsagadar]

Alright - now I did understand all the magic that is going on here ;-)

Could have helped to use a different commit message than "Skip pytest in pre-commit.ci" - something like

Activate pre-commit.ci

Use https://pre-commit.ci to run pre-commit checks and automatically fix
detected issues.

Disables pytest in pre-commit.ci because they won't pass.

[tsagadar]

Just looked at the report as well - love this integration.

[b-rowan]

Alright - now I did understand all the magic that is going on here ;-)

Could have helped to use a different commit message than "Skip pytest in pre-commit.ci" - something like

Activates pre-commit.ci

Use https://pre-commit.ci to run pre-commit checks and automatically fix

detected issues.

Disable pytest in pre-commit.ci because they won't pass.

Still getting the hang of when to use longer commit messages and for what.