pre-commit: make hooks self contained + ci config #11226
Conversation
k8s-ci-robot
added
kind/cleanup
k8s-ci-robot
added
the
size/L
VannTen
force-pushed
the
cleanup/pre-commit-hooks
branch
5 times, most recently
from
767b352 to
9ac1fc0
Compare
VannTen
force-pushed
the
cleanup/pre-commit-hooks
branch
from
9ac1fc0 to
7ec99b4
Compare
|
Oh thanks, good catch !
|
VannTen
force-pushed
the
cleanup/pre-commit-hooks
branch
4 times, most recently
from
1d8d09f to
5ffb242
Compare
| generate-pre-commit: | ||
| image: 'mikefarah/yq@sha256:bcb889a1f9bdb0613c8a054542d02360c2b1b35521041be3e1bd8fbd0534d411' | ||
| stage: build | ||
| before_script: [] | ||
| script: | ||
| - > | ||
| yq -r < .pre-commit-config.yaml '.repos[].hooks[].id' | | ||
| sed 's/^/ - /' | | ||
| cat .gitlab-ci/pre-commit-dynamic-stub.yml - > pre-commit-generated.yml | ||
| artifacts: | ||
| paths: | ||
| - pre-commit-generated.yml | ||
|
|
||
| run-pre-commit: | ||
| stage: unit-tests | ||
| trigger: | ||
| include: | ||
| - artifact: pre-commit-generated.yml | ||
| job: generate-pre-commit | ||
| strategy: depend |
There was a problem hiding this comment.
Awesome, that's nice factoring and a way to keep the jobs running in parallel!
|
/approve |
k8s-ci-robot
added
the
lgtm
| - yamllint --strict . | ||
| except: ['triggers', 'master'] | ||
| generate-pre-commit: | ||
| image: 'mikefarah/yq@sha256:bcb889a1f9bdb0613c8a054542d02360c2b1b35521041be3e1bd8fbd0534d411' |
There was a problem hiding this comment.
HI @VannTen
To make the image more readable, is it ok to change the mikefarah/yq@sha256:bcb889a1f9bdb0613c8a054542d02360c2b1b35521041be3e1bd8fbd0534d411
to mikefarah/yq:4.44.1-githubaction
| pre-commit: | ||
| tags: | ||
| - light | ||
| image: 'ghcr.io/pre-commit-ci/runner-image@sha256:aaf2c7b38b22286f2d381c11673bec571c28f61dd086d11b43a1c9444a813cef' |
There was a problem hiding this comment.
Is it more readable by changing the ghcr.io/pre-commit-ci/runner-image@sha256:aaf2c7b38b22286f2d381c11673bec571c28f61dd086d11b43a1c9444a813cef
to ghcr.io/pre-commit-ci/runner-image:2024-05-11-4bb81b2-full
|
I specifically used hashes instead of tag for immutability (even though the tag are supposed to be immutable) and integrity.
Besides, it's not like the tag names are that much more readable ...
|
|
Yes, using sha256 is a good practice. |
This pre-commit does not require prerequisite on the host, making it easier to run in CI workflows.
This way, the hook is self contained and does not depend on a previous virtualenv installation.
- ansible-syntax-check - tox-inventory-builder - jinja-syntax-check
- Remove dependency of pydblite which fails to setup on recent pythons - Discard shell script and put everything into pre-commit
- markdownlint (manual fix) - end-of-file-fixer - requirements-txt-fixer - trailing-whitespace
client9/misspell is unmaintained, and has been forked by the golangci team, see client9/misspell#197 (comment). They haven't yet added a pre-commit config, so use my fork with the pre-commit hook config until the pull request is merged.
Use gitlab dynamic child pipelines feature to have one source of truth for the pre-commit jobs, the pre-commit config file. Use one cache per pre-commit. This should reduce the "fetching cache" time steps in gitlab-ci, since each job will have a separate cache with only its hook installed.
Use a style file as recommended by upstream. This makes for only one source of truth. Conserve previous upstream default for MD007 (upstream default changed here markdownlint/markdownlint#373)
VannTen
force-pushed
the
cleanup/pre-commit-hooks
branch
from
5ffb242 to
37d824f
Compare
k8s-ci-robot
removed
the
lgtm
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/approve |
|
Thanks @VannTen |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ant31, VannTen, yankay The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/cherrypick release-2.25 |
|
@tico88612: new pull request created: #11359 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/cherrypick release-2.25 |
|
@tico88612: new pull request could not be created: failed to create pull request against kubernetes-sigs/kubespray#release-2.25 from head k8s-infra-cherrypick-robot:cherry-pick-11226-to-release-2.25: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for k8s-infra-cherrypick-robot:cherry-pick-11226-to-release-2.25."}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request","status":"422"} In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
This makes all the pre-commit hooks self contained (=no need for a venv) which should help their setup on CI (such as pre-commit CI)
Also disable the autofix feature of pre-commit.ci.
Even if we don't use pre-commit.ci in the end, this should make it easier to run it in Gitlab CI
Which issue(s) this PR fixes:
Fixes #11211
Special notes for your reviewer:
aab5cdc is a lot of changes, but it's mostly autofix from pre-commit.
08e2a82 contains the gitlab-ci pre-commit integration.
The rest is just pre-commit hooks fix and conversion of script to actual pre-commits hooks.
Does this PR introduce a user-facing change?:
/label tide/merge-method-merge