forked from heavy-duty/ceremony
Merge pull request #146 from codex-bot-andresmgsl/build/145-review-mechanics-doctrine
docs: teach reviewers the queue mechanics
This commit is contained in:
commit
0fcd818396
2 changed files with 39 additions and 0 deletions
38
REVIEWER.md
38
REVIEWER.md
|
|
@ -16,6 +16,11 @@ The machine reads only your **verdict**; humans read your reasons.
|
|||
their discretion. Anything blocking — including a question whose answer
|
||||
gates your approval — is **request changes**, saying exactly what
|
||||
unblocks it.
|
||||
- **Name what you could not verify, in the verdict body.** Say which checks
|
||||
you could not run and why, and what you relied on instead: CI, reading, or
|
||||
a narrower probe. An unstated environment gap reads as coverage — exactly
|
||||
the blind spot Kimi's [crew report](https://github.com/heavy-duty/crew/blob/main/kimi-bot-andresmgsl/assessment.md)
|
||||
identified for boxes without `node` or `shellcheck`.
|
||||
- An approval you would not defend to the human is a defect. You are not
|
||||
being asked to be agreeable; you are being asked to be right.
|
||||
|
||||
|
|
@ -32,6 +37,13 @@ In order of authority:
|
|||
repo: its own CONTRIBUTING plus ceremony's README). A change that
|
||||
"simplifies away" a constraint gets request-changes with a link to the
|
||||
incident that made the rule.
|
||||
- **Verify a pinned consumer at its pin, not ceremony's `main`.** Every
|
||||
option, trigger, config key, and unmarked documentation claim must exist
|
||||
at that ref; run the pinned tool against the proposed config or read the
|
||||
tagged file. On [box#164](https://github.com/heavy-duty/box/pull/164),
|
||||
`0.1.0`'s `load_config` rejected `triage-actors=...` with
|
||||
`malformed label row` and `exit=1`. CI green on a conversion PR proves
|
||||
nothing about the new config: the base branch's workflow is what ran.
|
||||
3. **The code itself** — correctness first, then tests (does the test plan's
|
||||
floor exist? do the failure cases actually fail?), then conventions.
|
||||
Changelog line present for behavior changes; comments carry why, not
|
||||
|
|
@ -57,6 +69,32 @@ saw Y" outranks one that says "this looks like it might".
|
|||
repo you have never heard of reaches you; a repo list finds only work in
|
||||
repos somebody thought to list.
|
||||
|
||||
## How you work the queue
|
||||
|
||||
- **Your queue is the API, not the search index.** Enumerate
|
||||
`requested_reviewers` from the pulls API, your reviews from
|
||||
`pulls/N/reviews`, and comments from `issues/N/comments`. Search lag left
|
||||
cast#143, incubator#25, and box#164 waiting, as Claude's
|
||||
[crew report](https://github.com/heavy-duty/crew/blob/main/claude-bot-andresmgsl/assessment.md)
|
||||
records: search is only a backstop that adds candidates, never evidence of
|
||||
no duty. `requested_reviewers` self-clears when you submit, so the endpoint
|
||||
shows what you owe now.
|
||||
- **Every write is one-shot, keyed to (you, PR, head SHA).** Put a fresh
|
||||
read and verify immediately around the mutation; a session-start check is
|
||||
insufficient. If verification says it landed, stop even when the CLI
|
||||
looked unhappy. This binds the `🔎` announce as much as the verdict:
|
||||
deduplicate all discovery paths before acting. Duplicate verdicts on
|
||||
[#26](https://github.com/heavy-duty/ceremony/pull/26),
|
||||
[#29](https://github.com/heavy-duty/ceremony/pull/29), and
|
||||
[#39](https://github.com/heavy-duty/ceremony/pull/39), and duplicate
|
||||
announces on [#32](https://github.com/heavy-duty/ceremony/pull/32), bought
|
||||
the rule; do not answer a double-post with a third comment.
|
||||
- **Review each head in a throwaway checkout; keep the main clone clean.**
|
||||
Use a detached worktree per PR head and remove it after the verdict. A
|
||||
crashed build corrupted Claude's build clone in 2026-07-22
|
||||
([crew report](https://github.com/heavy-duty/crew/blob/main/claude-bot-andresmgsl/knowledge.md));
|
||||
running another tree in the clone you keep risks the whole box.
|
||||
|
||||
## What you do not do
|
||||
|
||||
- **Re-litigate the spec.** The issue's decisions were made in triage and,
|
||||
|
|
|
|||
1
changelog.d/145.md
Normal file
1
changelog.d/145.md
Normal file
|
|
@ -0,0 +1 @@
|
|||
- REVIEWER.md now carries the review mechanics every box had been re-deriving from an incident: the queue comes from the API and not the search index, every write is one-shot per (reviewer, PR, head), heads are reviewed in throwaway checkouts, a pinned consumer's config is verified at its pin, and a verdict names the checks its box could not run (#145).
|
||||
Loading…
Reference in a new issue