ci: one workflow, pre-commit first, and a single required check - #47
Merged
Conversation
Every Go repo under this tree had drifted into its own arrangement of the same four ideas, so this settles on the shape sh arrived at in blairham/sh#53 and applies it everywhere. Four things change. **One workflow.** Pre-commit and tests were separate files in some repos and folded together in others; a merge gate spread over two workflows is two places to keep in sync and neither one tells you whether the branch is mergeable. Everything that gates a merge now lives in ci.yml. **Pre-commit runs for every commit that reaches GitHub** — every push to a pull request, draft or not, and every push to main. It is the only job that looks at every file rather than at Go, and a secret committed to a draft is committed. Everything else waits on it: the cheap universal checks should fail before the expensive question is asked. **Tests are not re-run on merge to main.** Branch protection requires the branch to be up to date, so for a squash merge the tree that lands on main is the tree that was already tested. Re-running it proves nothing and delays the next merge. **golangci-lint runs through `go tool`.** The action's `version:` was a second pin next to go.mod's tool block, and the two had drifted apart — some repos pinned v2.12.2, some asked for `latest`, one took the action's default. CI now runs the version go.mod pins, which is the version `make lint` runs locally. Docs-only pull requests skip the code jobs, but they skip them at the *job* level rather than through a workflow `paths:` filter: a required check that never runs reports as pending forever, not as passed, which would make a prose-only pull request unmergeable. Anything unrecognised counts as code, so when the classifier is wrong it is wrong in the direction of running the tests. The single required check is ci-ok. Requiring the jobs by name does not survive this shape — a job skipped by the prose guard never reports, and a matrix job skipped by `if:` never expands, so a policy naming "test (ubuntu-latest)" waits for it forever.
The job carried `name: CI OK`, which is the string GitHub publishes as the check name — so branch protection would have had to require "CI OK" here and "ci-ok" in koi-shell, which already requires the id. One required check name across every repo is the entire point of the gate, so the display name goes and the id stands.
Two fixes to the pre-commit and changes jobs, both found by the first run of this workflow. The detector diffed against `origin/<base_ref>`, a ref actions/checkout does not necessarily leave behind. When it is absent `git diff` fails and prints nothing — and an empty diff is indistinguishable from "only prose changed", so the classifier concluded prose and skipped every test. A gate whose failure mode is silently skipping the tests is worse than no gate. It now diffs against the pull request's base sha, checks the commit is actually present first, and runs everything when it is not. The pre-commit job also gets the module cache. `go-mod-tidy-repo` resolves the whole dependency graph, and doing that uncached took two and a half minutes in koi-shell and then failed outright on a transient proxy.golang.org stream error.
The push trigger carried only the pre-commit job, on the reasoning that branch protection keeps branches up to date so the tested tree is the tree that lands. That is not quite true of a squash merge: the branch was tested before the squash, main can move under it afterwards, and the commit that ends up on main is one no pull request ever built. The merge result now gets its own run rather than inheriting the branch's green tick. Three things follow from it. The change detector runs on a push as well, diffing against `github.event.before` — the commit main pointed at — so the diff is the merge itself. It still treats a missing base commit as "run everything". Drafts keep skipping the code jobs; a push to main never does. Concurrency no longer cancels in-progress runs on main. A superseded push to a pull request is not worth finishing, but a run on main is the record of whether that merge was good, and cancelling it leaves the question open.
Linting was in two places with two different jobs to do, and the CI half was about to become a no-op. This settles it: the hook reports your change, the Lint job reports the pull request's change, and nothing lints the whole backlog on every commit. The hook stays `golangci-lint` — the `--new-from-rev HEAD` variant. It runs on every commit and should report what you just wrote, not the repository's history. Measured, the choice costs nothing either way: `pass_filenames` is false, so golangci type-checks the whole module regardless and `--new-from-rev` only filters what it prints — 0.82s against 0.93s on a warm cache. The reason to prefer it is the signal, not the second. `--new-from-rev HEAD` is useless on a runner, though: it compares the working tree against HEAD, and a CI checkout is clean, so it reports nothing while looking configured. So CI skips that one hook and the Lint job does the equivalent properly, diffing from the base *commit* — the pull request's base sha, or on a merge the commit main pointed at. Not `origin/<base_ref>`: actions/checkout does not reliably leave that ref behind. A missing base commit falls back to linting everything, because linting nothing is the one outcome worth ruling out. `golangci-lint-fmt` is no longer skipped in CI. Formatting is absolute rather than relative to a diff, and it is the same second of work.
Removes the separate lint job and the SKIP that suppressed the lint hooks. golangci-lint runs in pre-commit, on every commit, reporting what the change introduced — one place, one configuration, no second pin to keep in step.
blairham
force-pushed
the
ci/standardize
branch
from
August 31, 2026 15:30
a0a5199 to
b916c5d
Compare
Removes the ci-ok job and gates per step rather than per job, which is the shape sh settled on. The gate existed because jobs were skipped wholesale on a prose-only change, and a skipped job never reports — so branch protection could not name the jobs themselves. Gating inside the job removes that constraint: the job always runs and always reports, and only the expensive steps are skipped. With that, the branch policy names the jobs directly and there is nothing to keep in step with a `needs` list. Every job that reads the detector now lists `changes` in its `needs`, since the output has to be reachable from the step conditions. Concurrency cancels in-progress runs again, on main as well as on pull requests, matching sh rather than carrying a local exception.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Settles every Go repo under this tree on the CI shape
sharrived at in blairham/sh#53, because they had each drifted into their own arrangement of the same four ideas.One workflow
Pre-commit and tests were separate workflow files in some repos and folded together in others. A merge gate spread over two workflows is two places to keep in sync, and neither one answers "is this branch mergeable?". Everything that gates a merge now lives in
ci.yml.Pre-commit runs for every commit that reaches GitHub
Every push to a pull request, draft or not, and every push to
main. It is the only job that looks at every file rather than at Go — whitespace, secrets, the toolchain pin — and a secret committed to a draft is committed. Everything elseneeds:it, so the cheap universal checks fail before the expensive question gets asked.Tests are not re-run on merge to
mainBranch protection requires the branch to be up to date before merging, so for a squash merge the tree that lands on
mainis the tree that was already tested. Re-running it proves nothing and delays the next merge.golangci-lint runs through
go toolThe action's
version:was a second pin sitting next togo.mod'stoolblock, and the two had drifted: some repos pinnedv2.12.2, some asked forlatest, one took the action's default. CI now runs the versiongo.modpins — the same onemake lintruns locally.Docs-only pull requests
They skip the code jobs, but at the job level rather than through a workflow
paths:filter. A required check that never runs reports as pending forever rather than as passed, which would make a prose-only pull request unmergeable. Anything unrecognised counts as code, so when the classifier is wrong it is wrong in the direction of running the tests.One required check:
ci-okRequiring the jobs by name does not survive this shape — a job skipped by the prose guard never reports, and a matrix job skipped by
if:never expands, so a policy namingtest (ubuntu-latest)waits for it forever.ci-okrunsif: always()and fails if any upstream job failed.Branch protection needs updating to require
ci-okonce this merges — that is what makes "no post-merge test run" safe.