ci: check generated code freshness#126
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
Looks like the generated-code check failed because |
iainmcgin
left a comment
There was a problem hiding this comment.
[claude code] Thanks — this closes a real gap (#95): today stale generated code only surfaces indirectly as compile errors in Check/Test, and a dedicated job with a clear "run task generate:all and commit the diff" message is a much better failure mode. The action SHA-pinning and the ${{ env.PROTOC_VERSION }} reuse are consistent with how the existing jobs are set up, and the CONTRIBUTING.md updates match the new behavior. A few points before merging:
1. Please pin the buf version in bufbuild/buf-action. With setup_only: true and no version, the job installs whatever the latest buf release is on the day it runs, so the job's behavior can drift over time independent of anything in the repo. The other toolchain inputs in this workflow are pinned (protoc via PROTOC_VERSION, actions by SHA); buf should follow suit — a version: input plus a comment noting it's the same buf used for local regeneration is enough.
2. Known flake mode worth a comment in the job. The workspace has no committed Cargo.lock, so cargo generate-lockfile resolves buffa — and, more importantly, prettyplease, which formats all generated output — to the latest compatible versions at run time. A new prettyplease (or buffa) release can therefore turn this job red on PRs that touched nothing, until someone regenerates and commits the diff. That's arguably the job doing its job (a maintainer regenerating locally would see the same diff), but it's worth a short comment in the workflow so whoever sees the failure understands why it can happen without any change to the protos or codegen. If you'd rather avoid that mode entirely we can discuss pinning options, but I don't think it needs to block this PR.
3. Optional simplification: the awk over Cargo.lock works, but cargo pkgid buffa (after cargo generate-lockfile) prints …buffa@<version> and is less sensitive to lockfile formatting; version="${$(cargo pkgid buffa)##*@}" would replace the whole block. Take it or leave it.
4. Timeout: task generate:all builds protoc-gen-connect-rust and the buffa plugins in release mode; on a cold cache that plus the checkout/setup steps may brush up against timeout-minutes: 15. If the first real run times out, bumping to 20–25 is fine — no need to change preemptively.
The job hasn't executed in CI yet (the workflow run needs maintainer approval, which we'll sort out on our side) — we'd want to see it green here before merging, since the interesting failure modes (sibling ../buffa layout, plugin builds, buf invocation) are exactly the parts that only show up in the runner environment. No action needed from you on that beyond keeping the branch as-is; we'll kick the run and follow up if anything in the job needs adjusting.
Pin the buf release installed by buf-action so the job's behaviour does not drift with new buf releases, and document at the job level that the unlocked buffa/prettyplease resolution can require a regeneration commit when those crates publish new releases.
|
[claude code] To save a round trip I've pushed a small commit to this branch (maintainer edit) applying points 1 and 2 from the review: the buf release installed by |
Summary
check-generated-codeCI job that runstask generate:allbuffacheckout from the version locked inCargo.lockCloses #95.
Verification
Environment:
task 3.50.0buf 1.69.0protoc libprotoc 33.5../buffachecked out atv0.6.0Commands:
task generate:allgit diff --exit-code -- conformance/src/generated examples/eliza/src/generated examples/multiservice/src/generated benches/rpc/src/generatedgit diff --check