Add make check target to verify generated code is up to date#29
Merged
Add make check target to verify generated code is up to date#29
make check target to verify generated code is up to date#29Conversation
Runs `./scripts/generate.sh` and fails if it produces a diff in the generated files, so CI can catch drift between the OpenAPI spec and the committed client. Pre-checks that the generated files have no uncommitted local changes (exits 2 with a distinct message) so a dirty working tree isn't misreported as generator drift. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switches the dirty-tree pre-check and the post-generate drift check from `git diff` to `git status --porcelain` so they cover: - staged changes (the previous `git diff --quiet` only saw unstaged changes against the index, so `git add`-ed generated files would silently bypass the pre-check) - untracked files (a brand-new `*_flags.gen.go` produced by a spec change would slip past `git diff --exit-code`) Also prints the offending paths via `git status --short` when drift is detected, so the failure message points at the files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@greptileai review |
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
make checktarget that runs./scripts/generate.shand fails with exit 1 if it produces a diff in the generated files (internal/api/client.gen.go,internal/api/property_names.gen.go,internal/cmd/*_flags.gen.go).Not a true dry run —
generate.shwrites to disk, somake checkwill overwrite the files and rely ongit diffto detect drift. A pure dry-run would require refactoring the script to write to a temp dir.Test plan
make checkexits 0.internal/api/client.gen.go,make checkexits 2 with the "uncommitted local changes" message.make checkexits 1 with the "out of date" message.🤖 Generated with Claude Code
Greptile Summary
Adds a
checkMake target that pre-checks for uncommitted local changes in generated files (exiting 2), runsgenerate.sh, then checksgit status --porcelainto detect drift (exiting 1). The use ofgit status --porcelaincorrectly captures both staged and working-tree changes relative to HEAD, resolving the previous concern aboutgit diff --quietcomparing against the index.Confidence Score: 5/5
Safe to merge — only a minor UX nit in an error message remains.
The implementation is logically sound:
git status --porcelaincorrectly detects staged, unstaged, and untracked changes relative to HEAD. The only remaining finding is a P2 wording improvement in the post-drift error message.No files require special attention.
Important Files Changed
checktarget usinggit status --porcelainto pre-check for dirty generated files and post-check for generator drift; logic is correct, one minor UX nit in the post-drift error message.Prompt To Fix All With AI
Reviews (4): Last reviewed commit: "Address PR review: detect staged changes..." | Re-trigger Greptile