diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index bb69812b..14ccbe6a 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,73 +1,33 @@ ## Outcome - + -## Issue and context +## Design and boundaries - + -## Architecture and key concepts +## Review focus - + -## How it works +1. `path`: Does ...? - +## Evidence and remaining gates -## Design decisions and tradeoffs + - +- Verified: +- Not yet verified: -## Security, failure, and operations - - - -## Review guide - -### Suggested order - - - -1. - -### Verify carefully - - - -- [ ] - -## Validation - - - -| Evidence | What it proves | -| -------- | -------------- | -| | | - -## Known limitations and follow-ups - - - -## Metadata checklist - -- [ ] Backlog issue linked with correct close/reference semantics -- [ ] Local backlog document linked -- [ ] Added to Project 5 (`Dotify sprints`) -- [ ] Project Priority, Track, Phase, Type, and Backlog doc mirror the issue -- [ ] Workflow status matches draft/review state -- [ ] Assignee set -- [ ] Applicable labels set -- [ ] Applicable milestone set, or confirmed none exists -- [ ] Reviewers requested when ownership is known -- [ ] Draft/ready state is intentional + diff --git a/docs/backlog/README.md b/docs/backlog/README.md index 2bcbee1a..49d4e27b 100644 --- a/docs/backlog/README.md +++ b/docs/backlog/README.md @@ -18,6 +18,9 @@ while `main` is the GitHub default branch, closing keywords do not close these issues automatically; the merge handoff must close the issue and set its Project item to Done explicitly. +The PR writing workflow is tracked in [`pr-review-quality.md`](pr-review-quality.md) +(#240); it changes review documentation only, not product behavior. + ## Product north star Dotify is not a Spotify clone. Dotify is a decentralized cultural social hub where music becomes a live social connector, artists retain sovereignty over catalog/access/royalties, and listeners can discover music through shared real-time presence. diff --git a/docs/backlog/pr-review-quality.md b/docs/backlog/pr-review-quality.md new file mode 100644 index 00000000..27c90d02 --- /dev/null +++ b/docs/backlog/pr-review-quality.md @@ -0,0 +1,26 @@ +# PR review quality + +Issue: #240 + +## Outcome + +Make Dotify PR descriptions easier to review repeatedly without losing the +architectural and operational knowledge they are meant to preserve. + +## Scope + +- Use four reviewer-facing sections: outcome, design and boundaries, review + focus, evidence and remaining gates. +- Keep issue linkage, design reasoning, security/failure boundaries, concrete + review questions, and proof proportional to the change's risk. +- Verify Project 5 and other GitHub metadata outside the narrative body. + +## Acceptance + +- The template prompts for every required part of the PR knowledge-sharing + contract without duplicating the same flow across sections. +- A small change can be described briefly; money, access, security, migration, + or deployment changes still carry their necessary detail. +- No runtime or deployment behavior changes. + +Historical PR descriptions and metadata automation are out of scope. diff --git a/docs/explanation/pull-requests-as-knowledge-sharing.md b/docs/explanation/pull-requests-as-knowledge-sharing.md index afc58d40..3ba24738 100644 --- a/docs/explanation/pull-requests-as-knowledge-sharing.md +++ b/docs/explanation/pull-requests-as-knowledge-sharing.md @@ -5,7 +5,8 @@ A Dotify pull request is both a change proposal and a compact engineering lesson. It should let a reviewer understand the problem, reconstruct the reasoning, inspect the implementation in a deliberate order, and maintain the -result without depending on undocumented author context. +result without depending on undocumented author context. Its description is a +decision guide, not a second changelog or a narration of the diff. A changelog answers "what changed?" A strong PR also answers: @@ -23,7 +24,12 @@ repeat the diff line by line. ## The required story -Every PR description must contain the following information. +Every PR description must convey the following information, but not through a +separate heading for every item. Use the four sections in the PR template: +Outcome, Design and boundaries, Review focus, and Evidence and remaining gates. +Scale the depth to the risk: a narrow presentation fix should be brief; a +financial or security change must explain its trust and failure boundaries. +Do not add filler such as "no alternatives" or "no risks" to satisfy a shape. ### Outcome @@ -36,31 +42,21 @@ Explain the original behavior, its user or system impact, why the work matters now, and the constraints inherited from Dotify's product and security model. Link the backlog issue and its local scope document. -### Architecture and concepts +### Design and boundaries Describe the important boundaries, components, ownership, and data or control -flow. Define concepts that may be unfamiliar, such as a read model, -stale-while-revalidate, reorg checkpoint, capability token, or host-only key -delivery. Use the smallest useful diagram when several components interact. +flow. Define unfamiliar concepts in plain language. Explain the decisive design +choice and meaningful alternative, if there was one. For changes involving +trust, money, or operations, state what is authoritative, how failure and +persistence work, and what operators must configure, monitor, or roll back. +Never imply guarantees the implementation cannot provide. Link a design note +or use a small diagram only when it clarifies a multi-component change. -### Design decisions and tradeoffs +### Review focus -Explain why the chosen design fits the ticket. Name meaningful alternatives -that were considered and why they were deferred or rejected. Record deliberate -limitations instead of presenting them as accidental omissions. - -### Security, failure, and operations - -State what is trusted, what is authoritative, where behavior fails closed, -which degraded modes remain available, what is persisted, and what operators -must configure or monitor. Never imply guarantees the implementation cannot -provide. - -### Code map and review guide - -Give reviewers an ordered path through the change. For each stage, explain what -they should learn and which invariants or risks they should verify. Point to -specific files, modules, endpoints, contracts, or migrations. +Give reviewers an ordered path through the change. Name the one to three +highest-value questions, each tied to a specific file, module, endpoint, +contract, or migration and a concrete invariant or regression risk. The guide must include concrete review prompts. "Please review" is not enough. Examples: @@ -71,16 +67,18 @@ Examples: - Are cache keys and ETags scoped to every response variant? - Does a retry duplicate a financial or irreversible action? -### Validation and residual risk +### Evidence and remaining gates Map tests and checks to the behaviors they prove. Separate automated evidence -from manual or production evidence. List known limitations, follow-up work, and -the condition that allows the linked issue to close. +from manual or production evidence. List unverified behavior, deliberate +limitations, follow-up work, and the condition that allows the linked issue to +close. Do not list every routine command when one sentence conveys the proof. ## Metadata contract Every applicable metadata field is part of the engineering record, not -administrative decoration. +administrative decoration. Set and verify these in GitHub; do not paste a +metadata checklist into the reviewer-facing description. - Add every PR to GitHub Project 5, `Dotify sprints`. - Link the backlog issue. Use `Closes #N`, `Fixes #N`, or `Resolves #N` when