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

[CI:DOCS] Add pre-commit (app) hook to check IMGSFX #364

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

cevich
Copy link
Member

@cevich cevich commented Jun 28, 2024

Intended for use by the pre-commit app, this hook keeps track of all IMG_SFX values pushed, failing when any duplicate is found. In the case of pushing to PRs that don't build CI VM images, the hook failure must be manually bypassed. Example .pre-commit-config.yaml:

---
repos:
  - repo: https://github.com/containers/automation_images.git
    rev: <tag or commit sha>
    hooks:
      - id: check-imgsfx

@cevich
Copy link
Member Author

cevich commented Jun 28, 2024

Manually tested with:

pre-commit try-repo . check-imgsfx --verbose --all-files --hook-stage pre-push

Correctly throws error:

Check IMGSFX for accidental reuse........................................Failed
- hook id: check-imgsfx
- duration: 0s
- exit code: 1

FATAL: 20240620t153000z-f40f39d13 has already been used
Please rerun 'make IMG_SFX'

If I then make IMG_SFX and stage it, running the same command returns:

Check IMGSFX for accidental reuse........................................Passed
- hook id: check-imgsfx
- duration: 0s

@cevich
Copy link
Member Author

cevich commented Jun 28, 2024

force-push: Renamed all IMGSFX references to IMG_SFX.

@cevich cevich requested a review from edsantiago June 28, 2024 15:40
@cevich cevich marked this pull request as ready for review June 28, 2024 15:40
@cevich
Copy link
Member Author

cevich commented Jun 28, 2024

@edsantiago PTAL, even if you don't use the pre-commit app, I'd appreciate a quick look-through for any typos I may have made.

After this merges, my intention is to open a followup with an example .pre-commit-config.yaml file specific to this repo. In addition to your neat IMG_SFX check, it will also validates yaml, EOF, trailing whitespace, shellcheck, and a few more. But I need a commit-id on main to reference for the new check-imgsfx.

Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pre-commit looks intriguing, thanks for the pointer. I can't verify the config. Some feedback on the sh part.

check-imgsfx.sh Outdated Show resolved Hide resolved
check-imgsfx.sh Outdated Show resolved Hide resolved
@cevich
Copy link
Member Author

cevich commented Jul 1, 2024

Force-push: Remove use of $HOME based storage of history. Add intended use/purpose comment block to script.

@edsantiago this is ready for final review/merging. I'll start working on a followup PR to add an example pre-commit configuration file.

check-imgsfx.sh Outdated Show resolved Hide resolved
Intended for use by [the pre-commit
app](https://pre-commit.com/#intro), this hook keeps track of all IMG_SFX
values pushed, failing when any duplicate is found.  In the case of
pushing to PRs that don't build CI VM images, the hook failure must be
manually bypassed.  Example `.pre-commit-config.yaml`:

```yaml
---
repos:
  - repo: https://github.com/containers/automation_images.git
    rev: <tag or commit sha>
    hooks:
      - id: check-imgsfx
```

Signed-off-by: Chris Evich <cevich@redhat.com>
@edsantiago edsantiago merged commit cfc18f0 into containers:main Jul 1, 2024
39 checks passed
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.

2 participants