0c880bfbb3
- Drop the concrete incident reference; the doc should stand on its own without pointing at one PR. - Reframe the three-part gate as per-repo branch-protection settings, not universal fact — CI-required, stale-review-dismissal, and auto-merge are each configurable, not hive law. - Reframe what auto-merge signals: the arming human already judged the PR sound at a coarse level and expects only minor tweaks past that point, not zero human review ever. A reviewer's bar is substantive doubt, not any nit.
3.1 KiB
The PR review gate
What a review verdict means, why a reviewer shouldn't wait on CI to submit one, and what "armed to auto-merge" actually signals about the human review that already happened.
The gate has (up to) three parts, and they're per-repo settings
Whether a PR can merge, and what counts toward "can", is configured per repo in its branch-protection settings — not a fact true of every hive or every repo. The pieces a repo can require:
- CI is green — the repo's required status checks pass on the PR's current head commit, if the repo requires any.
- Requested reviews are
APPROVED— and whether a review is invalidated by a later commit ("stale") is itself a setting (Forgejo's "dismiss stale approvals" branch-protection option), not universal behavior. - Someone with write access has armed the PR to merge — a manual merge once the required conditions hold, or Forgejo's auto-merge (merges automatically the moment the other required conditions are met).
Where these are required, they're independent of each other. A reviewer only ever owns the review-approval piece — CI resolves (or doesn't) on its own regardless of what a review says, and merge-arming is someone else's call.
Reviewers: submit the verdict, don't gate it on CI
Submit hive-forge pr-reviews <pr> --approve or --request-changes
as soon as you've finished checking the diff — don't hold it back
waiting for CI to go green first. Mention CI's current state in the
review body if it's relevant (e.g. "approving; nix flake check is
still running"), but don't gate the formal verdict on it: CI isn't a
signal a reviewer waits on, it's a separate condition that resolves
independently.
What arming auto-merge actually means
Auto-merge isn't "no human ever looked at this." Whoever arms it has already judged the PR sound at a coarse level — the signal it sends is roughly "apart from maybe minor tweaks a reviewer can still catch, I think this is fine." That's the human-in-the-loop step, and it already happened. No large changes are expected to surface after that point — a reviewer's job past that point is to flag it if one does, not to assume none ever will.
The practical consequence for a reviewer: on a repo where auto-merge
may already be armed before your review lands, a plain APPROVED can
be the last step before the merge actually happens, with no further
review pass after yours. That's a reason to actually finish checking
before approving — not a reason to hesitate over every small thing.
Genuine, substantive doubt (a claim you haven't verified, a real
correctness question) is worth a request-changes or a clarifying
comment before approving; a stray style nit isn't the same category.
Re-review after a repo requires it
Where a repo dismisses stale approvals on a new commit, a review you already gave stops counting the moment a follow-up commit lands — even one whose message reads as trivial ("just a wording fix", "just trimming comments"). Re-diff and re-verify before submitting a fresh verdict; don't take a small-sounding commit message as an accurate description of the diff.