Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

confine pre-commit to stages #3940

Merged
merged 7 commits into from
Oct 27, 2023

Conversation

davidculley
Copy link
Contributor

@davidculley davidculley commented Oct 10, 2023

Description

See https://pre-commit.com/#confining-hooks-to-run-at-certain-stages

If you are authoring a tool, it is usually a good idea to provide an appropriate stages property. For example a reasonable setting for a linter or code formatter would be stages: [pre-commit, pre-merge-commit, pre-push, manual].

Checklist - did you ...

  • Add an entry in CHANGES.md if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

See https://pre-commit.com/#confining-hooks-to-run-at-certain-stages

> If you are authoring a tool, it is usually a good idea to provide an appropriate `stages` property. For example a reasonable setting for a linter or code formatter would be `stages: [pre-commit, pre-merge-commit, pre-push, manual]`.
@JelleZijlstra JelleZijlstra added skip news Pull requests that don't need a changelog entry. and removed skip news Pull requests that don't need a changelog entry. labels Oct 16, 2023
@JelleZijlstra
Copy link
Collaborator

Thanks. Could you add a changelog entry since this changes user-visible behavior?

@davidculley
Copy link
Contributor Author

Thanks. Could you add a changelog entry since this changes user-visible behavior?

Done. It seems now that, for some reason, the documentation cannot be generated for the Windows build.

CHANGES.md Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
davidculley and others added 3 commits October 25, 2023 18:34
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@Stiffler911
Copy link

Thanks. Could you add a changelog entry since this changes user-visible behavior?

Done. It seems now that, for some reason, the documentation cannot be generated for the Windows build.

Why does this happen?

@JelleZijlstra
Copy link
Collaborator

Some bug that we didn't figure out. Apparently it's fixed now, let me retrigger CI.

@JelleZijlstra JelleZijlstra merged commit 7686989 into psf:main Oct 27, 2023
38 checks passed
@davidculley
Copy link
Contributor Author

Thanks for your help @JelleZijlstra 🙂

@davidculley davidculley deleted the confine-precommit-to-stages branch October 30, 2023 21:36
@martin-thoma
Copy link

martin-thoma commented Nov 21, 2023

When I now try to commit, I get:

pre_commit.clientlib.InvalidManifestError: 
==> File /home/martin/.cache/pre-commit/repolc31_gpb/.pre-commit-hooks.yaml
==> At Hook(id='black')
==> At key: stages
==> At index 0
=====> Expected one of commit, commit-msg, manual, merge-commit, post-checkout, post-commit, post-merge, post-rewrite, prepare-commit-msg, push but got: 'pre-commit'

reverting to 23.10.1 solves the issue for me.

edit: I think I'll open an issue.

@davidculley
Copy link
Contributor Author

When I now try to commit, I get:

pre_commit.clientlib.InvalidManifestError: 
==> File /home/martin/.cache/pre-commit/repolc31_gpb/.pre-commit-hooks.yaml
==> At Hook(id='black')
==> At key: stages
==> At index 0
=====> Expected one of commit, commit-msg, manual, merge-commit, post-checkout, post-commit, post-merge, post-rewrite, prepare-commit-msg, push but got: 'pre-commit'

reverting to 23.10.1 solves the issue for me.

edit: I think I'll open an issue.

@MT-Cash: This should be fixed by upgrading pre-commit:

pip install --upgrade pre-commit

I think you need to use pre-commit 3.2.0 or newer. See https://github.com/pre-commit/pre-commit/releases/tag/v3.2.0

Or here:

new in 3.2.0: The values of stages match the hook names. Previously, commitpush, and merge-commit matched pre-commitpre-push, and pre-merge-commit respectively.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants