Skip to content

Add make check target to verify generated code is up to date#29

Merged
leo-notte merged 2 commits intomainfrom
add-make-check
Apr 17, 2026
Merged

Add make check target to verify generated code is up to date#29
leo-notte merged 2 commits intomainfrom
add-make-check

Conversation

@leo-notte
Copy link
Copy Markdown
Contributor

@leo-notte leo-notte commented Apr 16, 2026

Summary

  • Adds a make check target that runs ./scripts/generate.sh and 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).
  • Pre-checks the working tree first: if those generated files already have uncommitted local changes, it exits 2 with a distinct message so a dirty tree isn't misreported as generator drift.

Not a true dry run — generate.sh writes to disk, so make check will overwrite the files and rely on git diff to detect drift. A pure dry-run would require refactoring the script to write to a temp dir.

Test plan

  • On a clean tree with up-to-date generated files, make check exits 0.
  • After manually editing internal/api/client.gen.go, make check exits 2 with the "uncommitted local changes" message.
  • After a spec change that would regenerate output, make check exits 1 with the "out of date" message.

🤖 Generated with Claude Code

Greptile Summary

Adds a check Make target that pre-checks for uncommitted local changes in generated files (exiting 2), runs generate.sh, then checks git status --porcelain to detect drift (exiting 1). The use of git status --porcelain correctly captures both staged and working-tree changes relative to HEAD, resolving the previous concern about git diff --quiet comparing 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 --porcelain correctly 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

Filename Overview
Makefile Adds a check target using git status --porcelain to 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.

Fix All in Claude Code

Prompt To Fix All With AI
This is a comment left during a code review.
Path: Makefile
Line: 83

Comment:
**Misleading post-drift message**

By the time this line is reached, `./scripts/generate.sh` has already run (line 80) and the files have already been updated on disk. Telling the user to "Run 'make generate'" is redundant and potentially confusing — they only need to commit the files that were just regenerated.

```suggestion
		(echo "Generated code is out of date. Run 'git add' and commit the regenerated files above." && git status --short -- internal/api/client.gen.go internal/api/property_names.gen.go 'internal/cmd/*_flags.gen.go' && exit 1)
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (4): Last reviewed commit: "Address PR review: detect staged changes..." | Re-trigger Greptile

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>
Comment thread Makefile Outdated
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>
@leo-notte
Copy link
Copy Markdown
Contributor Author

@greptileai review

@leo-notte leo-notte merged commit 90703a8 into main Apr 17, 2026
3 checks passed
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.

1 participant