docs: refresh repository agent guidelines#734
Conversation
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAGENTS.md was rewritten with repository contribution rules, updated implementation conventions, and an ordered validation checklist covering formatting, vetting, tests, builds, linting, security scanning, and diff hygiene. ChangesRepository guidance
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@AGENTS.md`:
- Line 57: Update the “Diff hygiene” guidance in AGENTS.md to validate the
complete staged and unstaged diff before committing or opening a pull request.
Replace the working-tree-only git diff --check command with git diff HEAD
--check, or explicitly instruct checking both cached and unstaged diffs.
- Line 47: Update the Formatting guidance in AGENTS.md to use make fmt-check as
the validation command. Retain go fmt ./... and make fmt only as remediation
commands, clearly distinguishing them from the non-mutating formatting check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
gnanam1990
left a comment
There was a problem hiding this comment.
APPROVE — accurate and well-structured. Verified every factual claim against Makefile on main:
make fmt-check,make fmt,make vet,make testall exist ✅- "
make lintruns fmt-check + vet, not golangci-lint" — matches exactly (lint: fmt-check vet) ✅ internal/perfbench/reports/holds only.gitkeep+README.md(0 tracked JSON), so the "don't commit reports" guidance is accurate and forward-looking ✅- Removing the obsolete
globs/alwaysApplyfrontmatter is correct — Zero doesn't interpret those ✅
CodeRabbit's two findings (use make fmt-check; use git diff HEAD --check) are already reflected in the current head — its suggestions diff against an older commit, so the CHANGES_REQUESTED is stale and safe to dismiss.
Optional non-blocking nit: the "don't commit reports" rule isn't enforced by .gitignore — an internal/perfbench/reports/*.json ignore entry would give it teeth. Fine as a fast-follow. Merge is kevin's call per the program gate.
gnanam1990
left a comment
There was a problem hiding this comment.
Re-approve (prior approval was auto-dismissed by 5fb01da). No regressions — the new commit just makes the formatting step non-mutating (make fmt-check as the check; go fmt/make fmt as remediation) and uses git diff HEAD --check. All factual claims still verified against the Makefile; both CodeRabbit findings now fully resolved. LGTM.
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Approving. I checked the guidance claims against the repo rather than taking them on faith, and they hold: make lint really is fmt-check plus vet only (so calling the pinned golangci run advisory and separate is right), make test really is the race-enabled suite, the zero-release build and smoke commands exist as documented, and the generated-reports rule matches the contract in internal/perfbench/reports/README.md. The frontmatter removal is better than cosmetic: nothing in the codebase parses globs or alwaysApply, and AGENTS.md is injected raw into the project context, so that block was pure prompt noise on every run. The scope, working-tree preservation, and CONTRIBUTING-first rules also line up with where the repo has been heading this week.
One nit, take it or leave it: "Never edit files under third_party/" survived the refresh, but there is no third_party directory in the repo. Since this PR exists to remove guidance that no longer matches reality, that line is a candidate for the same treatment. Not blocking on it.
Summary
Scope
Documentation only. This pull request changes AGENTS.md and no other file.
Verification
Summary by CodeRabbit