singularity/singularity-forge
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

docs(contributing): define execution-based review validation standard (#2364)

Browse source 
* docs(contributing): define execution-based review validation standard

Expand the Review process section to make explicit that reviewers are
expected to build and run tests locally — not just read the diff. Also
codifies what contributors must provide (regression tests for bug fixes,
failure-path tests for features) to unblock review.

Previously the section offered only logistics (PR size, response etiquette).
This adds the missing standard for what "reviewed" actually means.

* docs(contributing): add worktree checkout as explicit reviewer step 0

The prior commit defined build + test execution as the review standard
but omitted the prerequisite: checking out the branch locally before
reviewing. Without it the list implied reviewers could run commands
without having the branch. Also adds the closing line that correctness
claims require completing all five steps.
This commit is contained in:
Jeremy McSpadden 2026-03-24 14:33:51 -05:00 • committed by GitHub
parent c5c75b0273
commit e5138c86df
1 changed files with 26 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
CONTRIBUTING.md
View file

@ -158,6 +158,32 @@ PRs go through automated review first, then human review. To help us review effi
- Respond to review comments. If you disagree, explain why — discussion is welcome.
- If your PR has been open for a while without review, ping in Discord. We're a small team and things slip.
### What reviewers verify
Reading a diff is not the same as verifying a change. Our review standard is execution-based, not static-analysis-based.
**What reviewers do:**
1. **Check out the branch** — check out the PR branch locally (or in a worktree). Don't review from the diff view alone.
2. **Build the branch** — run `npm run build`. A diff that doesn't compile is not reviewable.
3. **Run the test suite** — run `npm test`. CI status is a signal, not a substitute for local verification.
4. **Trace root cause for bug fixes** — confirm the diff addresses the root cause described in the issue, not just the symptom.
5. **Check for a regression test** — bug fixes must include a test that would have caught the original bug. If it's absent, the fix is incomplete.
Only after completing these steps should a reviewer make claims about correctness.
**What "looks right" means:**
"Looks right" is the starting point for review, not the conclusion. "The tests pass" only means the tests pass — not that the claimed bug is fixed or the feature works as described. A well-written commit message on a broken change is still a broken change.
### What contributors must provide to unblock review
- **Bug fixes** — include a regression test. A fix without a test is an assertion, not a proof.
- **Features** — include tests covering the primary success path and at least one failure path.
- **Behavior changes** — update or replace any existing tests that cover the changed behavior. Don't leave passing-but-wrong tests in place.
If your PR claims to fix issue #N, reviewers will verify the fix addresses the root cause described in #N — not just that CI is green.
## Local development
```bash