docs(contributing): rename Manual Verification to Verification, make it a merge gate
The old name implied the list was only for by-hand steps, which sat badly with the fact that most entries in practice are programmatic — the test suite, `fish_indent`, `docs/verify-manual.py`, CI. Dropping "Manual" lets one list carry both kinds, and gives the checkbox state real meaning: - **Checked** — verified, whether it ran programmatically or the author performed it by hand. - **Unchecked** — an outstanding manual check the reviewer still has to perform. Left visible rather than dropped. - **The list is the merge gate.** A PR isn't merged until every box is checked; added as an explicit rule under Branching & Pull Requests alongside the other merge rules. Because unchecked boxes now block the merge, the guidance also states that only resolvable checks belong here — one nobody can run would block the PR indefinitely. Genuinely unverifiable caveats and assumptions go in `## Notes`, which already existed in the optional-sections list. Incidentally realigns with PRs 121-122, which had already shortened the heading to `## Verification`; that part of the recent drift was right.
This commit is contained in:
@@ -8,7 +8,7 @@
|
|||||||
|
|
||||||
Keep the two headings below, in this order, and delete these comments.
|
Keep the two headings below, in this order, and delete these comments.
|
||||||
Optional sections may be added between them (see the note above
|
Optional sections may be added between them (see the note above
|
||||||
"Manual Verification").
|
"Verification").
|
||||||
-->
|
-->
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
@@ -28,8 +28,8 @@
|
|||||||
site build only.
|
site build only.
|
||||||
|
|
||||||
If this PR resolves a tracked issue, close it with a trailing line at the
|
If this PR resolves a tracked issue, close it with a trailing line at the
|
||||||
end of this section — not at the very bottom, since Manual Verification
|
end of this section — not at the very bottom, since Verification is
|
||||||
is always last:
|
always last:
|
||||||
|
|
||||||
Closes #42
|
Closes #42
|
||||||
|
|
||||||
@@ -64,13 +64,25 @@
|
|||||||
reasonably ask why the obvious approach was rejected.
|
reasonably ask why the obvious approach was rejected.
|
||||||
-->
|
-->
|
||||||
|
|
||||||
## Manual Verification
|
## Verification
|
||||||
|
|
||||||
<!--
|
<!--
|
||||||
Reproducible steps a reviewer can run themselves, as a checkbox list.
|
Every check this change needs, as a checkbox list. Always the last
|
||||||
Run them first, then check each box — an unchecked box means it was NOT
|
section.
|
||||||
verified, and should be called out as such rather than left silently
|
|
||||||
blank. This section is always last.
|
A CHECKED box means verified — either it ran programmatically (test
|
||||||
|
suite, linter, docs verifier, CI) or the author performed it by hand and
|
||||||
|
confirmed the result. Check these off before opening the PR.
|
||||||
|
|
||||||
|
An UNCHECKED box is an outstanding manual check the reviewer still has to
|
||||||
|
perform. Leave anything you could not verify yourself unchecked rather
|
||||||
|
than dropping it, so it stays visible.
|
||||||
|
|
||||||
|
**This list is the merge gate: the PR does not merge until every box is
|
||||||
|
checked.** So only put things here that can actually be resolved — a
|
||||||
|
check nobody is able to run blocks the PR forever. Genuinely unverifiable
|
||||||
|
caveats, assumptions, and known limitations belong in a `## Notes`
|
||||||
|
section instead, where they inform the review without gating it.
|
||||||
|
|
||||||
- One check per line, imperative, with the exact command in backticks
|
- One check per line, imperative, with the exact command in backticks
|
||||||
and the expected result stated.
|
and the expected result stated.
|
||||||
|
|||||||
+17
-6
@@ -48,6 +48,9 @@ assumes you *do* have push access (maintainers, regular contributors).
|
|||||||
branching doesn't touch the working tree.)
|
branching doesn't touch the working tree.)
|
||||||
- **Merge target is `main`, via PR.** Contributors open the PR; the repo
|
- **Merge target is `main`, via PR.** Contributors open the PR; the repo
|
||||||
owner merges it. Don't merge your own PR.
|
owner merges it. Don't merge your own PR.
|
||||||
|
- **Don't merge until the `## Verification` checklist is fully checked.**
|
||||||
|
Unchecked boxes are outstanding manual checks, not decoration. See
|
||||||
|
[Pull request descriptions](#pull-request-descriptions) below.
|
||||||
- **Keep feature branches focused.** If you stumble onto something unrelated
|
- **Keep feature branches focused.** If you stumble onto something unrelated
|
||||||
to your current task while working (a pre-existing bug, a stray cleanup),
|
to your current task while working (a pre-existing bug, a stray cleanup),
|
||||||
don't fold it into the same commit or PR. Handle it with one of these,
|
don't fold it into the same commit or PR. Handle it with one of these,
|
||||||
@@ -77,15 +80,23 @@ description box when you open a PR. Every PR carries, in this order:
|
|||||||
`Scope note`, `Opinionated guard (C1-C6)`), and skip them entirely for a
|
`Scope note`, `Opinionated guard (C1-C6)`), and skip them entirely for a
|
||||||
straightforward change. A breaking change (title ending in `!` before
|
straightforward change. A breaking change (title ending in `!` before
|
||||||
the colon) must include `## ⚠️ Breaking Change` with the migration path.
|
the colon) must include `## ⚠️ Breaking Change` with the migration path.
|
||||||
- **`## Manual Verification`** — always last. Reproducible steps as a
|
- **`## Verification`** — always last. Every check this change needs, as a
|
||||||
checkbox list, each with the exact command and its expected result. Run
|
checkbox list, each with the exact command and its expected result.
|
||||||
them before opening the PR and check the boxes; a box left unchecked
|
A **checked** box means verified, whether programmatically (test suite,
|
||||||
means that step was *not* verified, so say so explicitly rather than
|
linter, docs verifier, CI) or by hand; check those off before opening the
|
||||||
leaving it silently blank.
|
PR. An **unchecked** box is an outstanding manual check the reviewer
|
||||||
|
still has to perform — leave anything you couldn't verify yourself
|
||||||
|
unchecked rather than dropping it.
|
||||||
|
|
||||||
|
**This list is the merge gate: a PR isn't merged until every box is
|
||||||
|
checked.** Only list checks that can actually be resolved — one nobody
|
||||||
|
can run blocks the PR indefinitely. Put genuinely unverifiable caveats,
|
||||||
|
assumptions, and known limitations in `## Notes` instead, where they
|
||||||
|
inform the review without gating it.
|
||||||
|
|
||||||
When a PR resolves a tracked issue, close it with a trailing `Closes #42`
|
When a PR resolves a tracked issue, close it with a trailing `Closes #42`
|
||||||
line at the end of `## Summary` — not at the very bottom of the body, since
|
line at the end of `## Summary` — not at the very bottom of the body, since
|
||||||
`## Manual Verification` is always last. `Fixes #N` and `Resolves #N` behave
|
`## Verification` is always last. `Fixes #N` and `Resolves #N` behave
|
||||||
identically. Repeat the keyword for each issue (`Closes #42, closes #43`); a
|
identically. Repeat the keyword for each issue (`Closes #42, closes #43`); a
|
||||||
bare `#43` is only a link and won't close anything. To point at a related
|
bare `#43` is only a link and won't close anything. To point at a related
|
||||||
issue that should stay open, drop the keyword and use `Refs #42`. Leave the
|
issue that should stay open, drop the keyword and use `Refs #42`. Leave the
|
||||||
|
|||||||
Reference in New Issue
Block a user