diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 75e0f6c..a945a32 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -7,11 +7,10 @@ approachable and respectful. > You can use [Wizard GitHub App][2] and [Wizard Browser Extension][1] to > simplify some of the workflows described in these Guidelines. -These conventions are being restructured into a -[rules system](./rules/README.md): small numbered files that each enforce one -checkable behavior. The Authoring rules (`DEV-0xx`) define how a rule is -written; the remaining categories migrate the sections below into rules. This -document threads them with the workflow narrative. +The enforceable conventions live in a [rules system](./rules/README.md): small +numbered `DEV-` files that each enforce one checkable behavior. This document is +the narrative guide over them, threading the rules with the workflow context. +When a rule and this guide ever disagree, the rule is the source of truth. ## Table of Contents @@ -116,32 +115,10 @@ rule that defines it: ### Review Process -#### Giving a Review - -If a PR is not ready to merge, you **must** use **Request Changes** (reject). Do -not leave a plain comment when rejection is warranted — comments do not block -merging, are not recorded as rejections, and prevent the author from -re-requesting a review. - -Use **Request Changes** (reject) for objective problems: - -- PR doesn't solve the stated problem. -- A bug is introduced. -- Code style is inconsistent. -- Required guidelines are violated. - -Use **Comment** for optional improvements or suggestions that should not block -the PR. - -#### Scout Approach - -When not actively working on a PR, look for PRs that need reviewers and offer -timely feedback to keep work moving. - -#### Code Quality - -Deliver bug-free software. Push back on subjective feedback — reviewers are a -final safety check, not a QA team. +- [DEV-410](./rules/DEV-410.md): reject with Request Changes for objective + problems +- [DEV-420](./rules/DEV-420.md): scout open PRs when idle +- [DEV-430](./rules/DEV-430.md): deliver bug-free work; review is a safety check --- diff --git a/docs/rules/DEV-410.md b/docs/rules/DEV-410.md new file mode 100644 index 0000000..e9447b3 --- /dev/null +++ b/docs/rules/DEV-410.md @@ -0,0 +1,30 @@ +--- +id: DEV-410 +title: "Reject With Request Changes for Objective Problems" +status: "active" +enforcement: "manual" +severity: "error" +depends_on: ["DEV-360"] +--- + +## Problem + +Leaving a plain comment on a PR that should be rejected does not block the +merge, is not recorded as a rejection, and stops the author from re-requesting +review. Real problems slip through as if they were optional. + +## Solution + +Match the review action to the finding. + +1. When a PR is not ready to merge, use **Request Changes** for objective + problems: it does not solve the stated problem, it introduces a bug, its code + style is inconsistent, or it violates a required guideline. +1. Use **Comment** for optional improvements or suggestions that should not + block the PR. + +### Acceptance Criteria + +- [ ] Objective problems get Request Changes, not a plain comment +- [ ] Optional suggestions use Comment +- [ ] A rejection names the objective problem it is based on diff --git a/docs/rules/DEV-420.md b/docs/rules/DEV-420.md new file mode 100644 index 0000000..0f1fafc --- /dev/null +++ b/docs/rules/DEV-420.md @@ -0,0 +1,27 @@ +--- +id: DEV-420 +title: "Scout Open PRs When Idle" +status: "active" +enforcement: "manual" +severity: "warning" +--- + +## Problem + +PRs wait and stall when no one picks them up for review. Work that is otherwise +ready sits idle because reviewers are focused only on their own changes. + +## Solution + +Reviewing is shared work, not something that happens only to your own PRs. When +your own work is not blocking you, spend that idle time clearing the queue so +ready changes do not wait on attention. + +Look for PRs that have no reviewer or are still waiting on one, and give +feedback while it still moves the work. A fast review on a ready PR is worth +more than a thorough one a day later. + +### Acceptance Criteria + +- [ ] Idle time is spent reviewing PRs that are waiting for a reviewer +- [ ] Feedback is given promptly enough to keep the work moving diff --git a/docs/rules/DEV-430.md b/docs/rules/DEV-430.md new file mode 100644 index 0000000..dfe6a2e --- /dev/null +++ b/docs/rules/DEV-430.md @@ -0,0 +1,32 @@ +--- +id: DEV-430 +title: "Deliver Bug-Free Work; Review Is a Safety Check" +status: "active" +enforcement: "manual" +severity: "error" +depends_on: ["DEV-410"] +--- + +## Problem + +Treating reviewers as the QA team pushes defects downstream, slows everyone, and +lets subjective debates block otherwise sound work. + +## Solution + +Own the quality of what you ship instead of outsourcing it to review. A reviewer +who has to find your bugs becomes a bottleneck, and a defect caught in review +costs more to fix than one you catch before opening the PR. + +Test and self-review the change until you believe it is bug-free, then request +review. Treat the reviewer as a final safety check on work you already trust, +not the QA pass that first exercises it. When feedback is subjective rather than +an objective problem (see [DEV-410](./DEV-410.md)), discuss and push back +instead of absorbing every preference as a required change, so sound work does +not stall on taste. + +### Acceptance Criteria + +- [ ] Work is delivered bug-free, not dependent on review to find defects +- [ ] Review is treated as a final safety check, not QA +- [ ] Subjective feedback is pushed back on rather than blocking the PR diff --git a/docs/rules/README.md b/docs/rules/README.md index e65a754..8d85863 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -60,8 +60,13 @@ What a pull request must satisfy before it merges. - [DEV-380](./DEV-380.md): enforce markdown lint on push with a pinned rumdl hook -The remaining categories are migrating from the -[Contributing Guidelines](../CONTRIBUTING.md) into rules. +### 4. Review + +How to review, and the quality bar work is held to. + +- [DEV-410](./DEV-410.md): reject with Request Changes for objective problems +- [DEV-420](./DEV-420.md): scout open PRs when idle +- [DEV-430](./DEV-430.md): deliver bug-free work; review is a safety check ## Rule file format