Conversation
The Makefile already defined the gate; nothing ran it but a person. CI now calls `make all` rather than copying its commands, so what passes locally and what CI judges cannot drift apart. Adds a `fmt` target — go vet says nothing about formatting — and folds it into `all`. Go version comes from go.mod so it is written in one place.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 978a297c99
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`files=$(gofmt -l .)` threw away gofmt's exit status. A file gofmt cannot parse prints nothing to stdout, so the gate read it as clean — and build, vet and test would miss it too if the file sat behind another platform's build tag. Check the status before the output. setup-go caches by default and looks for a go.sum. There is none and there never will be, so every run warned about it. Say `cache: false` and the comment claiming no cache becomes true.
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
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.
Summary
The Makefile already defined this project's quality gate, but nothing ran it except a person.
Makefileeven said so out loud — the-racedefault was justified with "there is no CI to run it anywhere else."This PR adds that CI. Every push to
mainand every pull request now gets an automatic pass/fail verdict, and the result is visible from the repository's first screen.The gate is
make all, not a list of commands copied out of the Makefile. If CI duplicated the commands, the two could drift and a local pass would stop meaning a CI pass.Changes
.github/workflows/ci.yml: on push tomainand on all pull requests, runmake allonubuntu-latestfmttarget that fails on unformatted files, and folded it intoall—go vetsays nothing about formatting, so drift would otherwise only surface in reviewgo.modviago-version-file, so the version is written in one placecontents: readtesttarget commentDeliberately not included: a multi-version Go matrix (
go.modrequires 1.26, so older toolchains cannot build this at all), a module cache (zero external dependencies),concurrency: cancel-in-progress, and a separatemake depsstep —TestGoListDepsProvesFusionIgnoranceandTestNoExternalDependenciesalready run undergo test ./....Validation
make fmt—OK: gofmt cleanmake all— fmt + build + vet +go test -race ./..., all packages pass locally in ~17sReview Focus
Three things only a real CI run can settle, and this PR's own run settles them:
actions/checkout@v4/actions/setup-go@v5resolve on the runnergo-version-file: go.modactually yields Go 1.26 therego listpass in a clean checkout with no module cache — they should, since there are no external dependencies to fetch, but this has never run outside a developer machineRisks / Notes
pkg/is untouched-racedefault thatpkg/engine/persist_test.go:712depends on is preservedmainis a separate manual step — the PRD leaves "display only vs. block" open