c20496604c
Moved 27 instances of periods and commas from outside closing quote marks to inside, following American-style punctuation placement. Changes span across ~15 documentation files including integration, process, scheduler, trust-boundary, and web-ui docs.
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 automerge" 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 automerge (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 (for example "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 automerge actually means
Automerge 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 automerge
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.