chore(scripts): gitignore built Go binaries - #2918
Conversation
Every scripts/<name>/ is a `main` package, so a build drops an executable named after its directory: `go build ./scripts/<name>/` leaves it in the working directory, and `go build ./...` from the root leaves one per package at the root. Neither has an extension, so nothing in the ignore file covered them, and a directory-wide `git add` stages a multi-megabyte Mach-O binary alongside the source without comment. Listed by name rather than by a glob for extensionless files. That glob would also hide a legitimately extensionless file added later, and a silently ignored source file is a worse failure than a binary `git status` still shows — the rule fails in the safer direction at the cost of a line when a package is added. Verified across all twelve paths: with the rule none is visible to git, without it all twelve are, and no tracked file becomes ignored. Fixes #2888.
@coderabbitai review |
|
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughUpdated 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitignore:
- Around line 34-38: Update the explanatory comment in .gitignore to remove the
incorrect `go build ./...` root-output claim. Document `go build
./scripts/<name>/` from the repository root for root-level output, and `go build
.` from within `scripts/<name>/` for directory-local output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ea61a8d4-123b-4ea7-b1e6-4fbb78f55c60
📒 Files selected for processing (1)
.gitignore
📜 Review details
🔇 Additional comments (1)
.gitignore (1)
40-55: LGTM!
`go build ./...` over multiple main packages discards the executables rather than writing one per package at the root, so the comment justified the root block with a command that produces nothing. Name the two forms that actually write output instead: `go build ./scripts/<name>/` from the root, and `go build .` from inside the package directory. The ignore entries themselves are unchanged and still correct.
CodeRabbit is not delivering at this head: the account-level review limit is reached ( |
Requesting a Codex review at the current head @codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7083d3e608
ℹ️ 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".
go test -c, a source-file build, and any GOOS=windows build write names the package-name rules do not match, so the directory-wide staging hazard remained for <name>.test, main, and every .exe variant.
All three Codex P2 findings fixed in @coderabbitai review |
|
✅ Action performedReview finished.
|
Readiness at
|
| check | result |
|---|---|
derived matrix — 6 packages × 6 output names × 2 locations, via git check-ignore |
0 of 72 uncovered |
ablation — remove exactly the 6 new rules (changed-lines=6, asserted) |
60 of 72 uncovered — only the 12 pre-existing literal names survive |
| restore | 0 of 72 |
negative control — git ls-files | git check-ignore --stdin |
0 tracked files newly ignored |
The predicted ablation figure (72 − 12) was stated before it was run and matched, so the rules are load-bearing rather than decorative.
2. Reviewed. CodeRabbit at this exact head — No actionable comments were generated in the recent review, reviewed range ending at 54b56962, updated 02:06:40Z after the 02:04:57Z authenticated request, no quota or service marker, zero finding sections. Codex's three P2 findings from the previous head are each fixed, answered with reproduction evidence, and resolved.
3. Tried and evaluated as a user. Not inferred from the matcher — I ran the real builds. All six forms (go build ./scripts/<name>/, go build ., go build main.go, go test -c, and the GOOS=windows variants of each) for two packages, from both the repository root and inside the package directory. The binaries were confirmed present on disk afterwards — main, main.exe, validate-concurrency-queue.exe, validate-concurrency-queue.test, validate-concurrency-queue.test.exe, and the validate-dr-signing equivalents — so the check was not vacuously passing over an empty tree. git status showed exactly one entry: the .gitignore edit itself.
Every one of those six forms produced an untracked file before this change. That is what the three findings were about, and it is now closed for all of them.
The compiled bridge binary was staged by a directory-wide add on a branch that predates the ignore rules merged in #2918. Removed here; the merge of main that follows brings those rules in so it cannot recur on this branch.
1 participant
Why
Building any of the Go tools under
scripts/drops a compiled binary next to the source, and nothing in the ignore file covered it. A directory-widegit addthen stages a multi-megabyte binary alongside the code without saying anything — the only thing preventing a committed binary was someone readinggit statuscarefully. That near-miss has now happened three times while working on #2854.What
Ignores the built binaries, at both places a build can leave them.
They are listed by name rather than matched by a general "files without an extension" rule. That rule would be self-maintaining, but it would also silently hide a legitimately extensionless file someone adds later — a worse failure than a stray binary, which
git statusat least still shows. The trade is one line when a new tool is added.Fixes #2888.