Skip to content

ci: check lychee-docker pre-commit version matches Cargo.toml#2231

Open
mre wants to merge 1 commit into
masterfrom
check-docker-precommit-version
Open

ci: check lychee-docker pre-commit version matches Cargo.toml#2231
mre wants to merge 1 commit into
masterfrom
check-docker-precommit-version

Conversation

@mre

@mre mre commented Jun 4, 2026

Copy link
Copy Markdown
Member

Follow-up to #2228. CI now fails if the docker tag in .pre-commit-hooks.yaml drifts from the Cargo.toml version, so the release PR catches it instead of someone noticing later.

Comment thread .github/workflows/ci.yml
Comment on lines +56 to +60
- name: Check lychee-docker pre-commit version matches Cargo.toml
run: |
version=$(grep -m1 '^version' Cargo.toml | sed -E 's/.*"(.*)".*/\1/')
grep -q "lycheeverse/lychee:$version" .pre-commit-hooks.yaml \
|| { echo "::error::Bump the lycheeverse/lychee docker tag in .pre-commit-hooks.yaml to $version"; exit 1; }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

How this works:
On release, release-plz bumps Cargo.toml, the release PR's CI turns red, and we have to manually fix the one line in .pre-commit-hooks.yaml.

It's not automatic, but very simple, self-documenting, and avoids the silent drift.

wdyt?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, I like it! It's appropriately simple given your use case. And if you're annoyed by this some time in future, you can still automate more 👍

@mre

mre commented Jun 4, 2026

Copy link
Copy Markdown
Member Author

@cbachhuber wdyt?

@katrinafyi katrinafyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like the idea.

Is there a cargo command we can use to get the version rather than grep the toml? Also, I think it could be more readable to use a bash if statement rather than ||.

How does the interaction with release-plz work, in particular how does it interact with the automatic rebase of the PR?

Are you able to push one commit fixing the pre commit version and release-plz will stack it's commits on top of that one? Or, does release-plz overwrite your commit and you'll need to bump pre-commit as the last commit before merging?

@mre

mre commented Jun 6, 2026

Copy link
Copy Markdown
Member Author

Is there a cargo command we can use to get the version rather than grep the toml? Also, I think it could be more readable to use a bash if statement rather than ||.

Oh, yeah, I think there is one. 🤔 Can't remember from the top of my head. Let me check.

How does the interaction with release-plz work, in particular how does it interact with the automatic rebase of the PR?

The check is very simple. It happens on every pull request. The Release-plz workflow only runs on pushes to master and on new releases, so it happens later.

Are you able to push one commit fixing the pre commit version and release-plz will stack it's commits on top of that one? Or, does release-plz overwrite your commit and you'll need to bump pre-commit as the last commit before merging?

As long as the build is green when we merge a PR, it's fine. Everything will work as before. The only difference is this sanity-check for the pre commit version. So if we bump the version in Cargo.toml, but forget to bump the pre commit version, it will be a CI error.

release-plz will always base its work on top of latest master, which means there are no problems unless we broke the build before (by merging a faulty PR) in which case the lint step on the release-plz PR will also fail, but we can fix that in a separate PR. After that, release-plz will update its own PR and everything will be green again. 😄

@mre

mre commented Jun 6, 2026

Copy link
Copy Markdown
Member Author

We could do

cargo metadata --format-version=1 --no-deps | jq -r '.packages[0].version'

wdyt?

@thomas-zahner

Copy link
Copy Markdown
Member

@mre So this yields to failing CI when encountering a version mismatch. Alternatively (or additionally?) we could create an action which automatically bumps the specified version. This is what we do with the table of contents for the README as well as (I think) two version references in our docs.

@mre

mre commented Jun 7, 2026

Copy link
Copy Markdown
Member Author

we could create an action which automatically bumps the specified version

yes, that would be better. 🤔 We can close this PR if we can somehow bump the version automatically. 👍

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