diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 00000000..0f5cd1c9 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,7 @@ + + +When reviewing a pull request, follow `REVIEW.md` at the repository root. +When writing code, follow `AGENTS.md`. diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 61087837..245c00e8 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -15,10 +15,15 @@ +## Revert + + + ## Checklist - [ ] Tests added/updated and `pytest` passes - [ ] `ruff`, `mypy`, and schema checks pass - [ ] Docs updated (or N/A) — see the docs-drift checklist in CONTRIBUTING.md +- [ ] Self-reviewed per "Before opening a PR" in `AGENTS.md` - [ ] No ad-hoc run artifacts committed - [ ] New files carry the SPDX license header (or are exempt in `.github/.licenserc.yaml`) diff --git a/AGENTS.md b/AGENTS.md index ca6b0e2c..0e154b0f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -126,3 +126,19 @@ Registries, bundled in the package (`dokimi/dokimi/`) so they ship in the wheel: branches, `dependabot/*` and `renovate/*`, are the exception). - When a change depends on an open PR, wait for it to merge or put both in one PR; PRs are not stacked on each other's branches. + +## Before opening a PR + +Review it yourself the way `REVIEW.md` asks a reviewer to, so the first round +finds little: + +- For each test of changed behaviour, break the code it covers (flip a + condition, drop a branch), confirm the test fails, then revert the break. +- For a bug fix, find every other site with the same bug class. Fix the ones + within this PR's scope; list the rest in the description or an issue. +- Where it applies, run the changed command on a bad input: missing file, + empty file, wrong shape, `None` where a value is expected, a Windows path. + Say in the description what you could not run. +- Check that the PR title, description, docstrings and changelog fragment say + what the code does (exit codes especially). +- Re-review your own fix commits during review with the same care. diff --git a/REVIEW.md b/REVIEW.md new file mode 100644 index 00000000..387d5be5 --- /dev/null +++ b/REVIEW.md @@ -0,0 +1,55 @@ + + +# Review guide + +For anyone reviewing a dokimi PR, person or agent. CI already enforces lint, +types, coverage, growth, import layers and test quality (`AGENTS.md`); do not +repeat what it checks. + +## What a defect looks like here + +- A command gives a wrong number, verdict or exit code, or crashes where it + should print a message. +- A test passes when the behaviour it names is broken. +- A doc, docstring, PR title or changelog fragment says something the code + does not do. +- A stored record is written or read with keys or a shape other than the + schema's. +- Behaviour differs on Windows or with a `None`, empty or missing input. + +## How to raise a finding + +- Name the file and line, and show the input that breaks it: a command, a + failing test or a mutation that the tests still pass. A finding found by + running code lands far more often than one found by reading. +- Name the bug class and every other site that has it, so one round fixes all + of them. +- Where a fix and a test are short, push them or put them in a suggestion + instead of describing them. +- Mark each finding must-fix or nit. At most three nits per review; skip the + style of `dokimi/changelog.d/` fragments, not their accuracy. +- A bug in code the PR did not change: mention it once as pre-existing. It + does not block the PR unless the change makes it worse. +- Do not repeat a finding already raised on the PR, unless its fix did not + hold. +- Start with the count of findings, and say "nothing to fix" when there is + nothing. + +## Rounds + +- After the first round, review what changed since and the code that calls + it. +- Re-run the original failing input on every fix, including fixes the + reviewer pushed. +- If a PR is still collecting must-fix findings after three rounds, step back + and agree on the approach in the thread before more commits. +- Close each thread with the commit that fixed it, or one line on why it is + not a defect. The reviewer closes their own thread where they can. + +## When the same comment keeps coming back + +Move it upstream: a test, a check in `make ci-local`, or a line in `AGENTS.md`, +so the next PR cannot reach review with it.