docs(pr): require user-perspective description and surface area (#1520)

* docs(pr): require user-perspective description and surface area

The previous template asked for Summary + Validation, which encouraged
code-perspective descriptions and let user-visible surface changes slip
past review unnoticed. Replace with:

- "Problem" — issue link + motivation
- "What users will see" — first-person user-visible effect
- "Surface area" — 9-item checklist (UI, shortcut, CLI/env, API/contract,
  extension point, i18n, top-level dependency, default behavior change, none)
- "Screenshots" — required when UI surface is checked, focused on the
  entry point users discover rather than the feature in isolation
- "Validation" — kept, retitled away from "Summary"

Authoritative rules added to AGENTS.md under a new "Pull request expectations"
section so external contributors' agents (Claude Code, Cursor, etc.) pick up
the requirement when reading the repo. CONTRIBUTING.md gets one pointer line
in "Commits & pull requests"; localized CONTRIBUTING variants (zh-CN, de, fr,
ja-JP, pt-BR) are left for follow-up translation PRs per the existing
docs-update workflow.

The existing "Fixes #" prompt is preserved verbatim — that template addition
from #1263 enforces PR-to-issue auto-linking and stays load-bearing.

* docs(pr): broaden dependency surface to dev deps as well

The "New top-level dependency" checkbox narrowed scope to runtime deps,
but CONTRIBUTING.md L239 says "No new top-level dependencies" without
limiting to runtime, and the AGENTS.md rule uses the same broad phrase.
A new devDependency (tool/test/build package) belongs in the same
bytes-vs-benefit explanation, so the checklist item should match.

* docs(pr): add Why and Bug fix verification; scope deps to root package.json

Three follow-up tweaks from review feedback:

1. Rename `## Problem` to `## Why` with a broader prompt that asks
   contributors to cover both their own use case (what made them write this
   PR today) and the pain being addressed. The old "Problem" framing only
   covered user-facing motivations and left no slot for the contributor's
   stake — a key signal for triaging external PRs.

2. New `## Bug fix verification` section between Screenshots and Validation,
   conditional on the PR being a bug fix. Surfaces the AGENTS.md
   "Bug follow-up workflow" red-spec requirement at PR-authoring time
   instead of leaving it implicit; asks for the test path and the
   red-on-main / green-on-branch confirmation.

3. Clarify the "New top-level dependency" checkbox to specify the **root**
   `package.json`. Without that word, contributors in a monorepo could read
   the check as applying to any workspace `package.json` (e.g. adding `react`
   to `apps/web/package.json` would be in scope) when CONTRIBUTING.md L239's
   "small on purpose" rule clearly meant root-level deps only.

AGENTS.md `## Pull request expectations` and CONTRIBUTING.md's pointer
line are updated to match the new section names and add the bug-fix
red-spec expectation.
This commit is contained in:
lefarcen 2026-05-13 15:28:05 +08:00 committed by GitHub
parent a98fd2e827
commit 6341b2677a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 72 additions and 1 deletions

View file

@ -3,8 +3,68 @@
Delete this whole block if the PR genuinely doesn't close any issue. --> Delete this whole block if the PR genuinely doesn't close any issue. -->
Fixes # Fixes #
## Summary ## Why
<!-- Why are you opening this PR? Cover two things:
- Your use case — what made you write this today? Did you hit it yourself
while building on top of OD, are you scratching an itch for a team, or
are you speculatively adding for others? All are fine, but say which.
- The pain being addressed — user-facing problem, technical debt, a prod
issue, or unblocking another change.
For non-trivial features, please open a discussion or issue first to align
on scope and UX before writing code. -->
## What users will see
<!-- Describe the change from a user's perspective, not in code terms. Examples:
- "Settings → AI Providers has a new 'Custom endpoint' field, off by default"
- "Right-clicking a design file shows a new 'Duplicate' option"
- "Default model changed from X to Y — existing users will notice on first launch"
- "New `od skills install <name>` subcommand"
Skip this section only if you check "None" below. -->
## Surface area
<!-- Check every box that applies. Reviewers use this to scope the review. -->
- [ ] **UI** — new page / dialog / panel / menu item / setting / empty state in `apps/web` or `apps/desktop` (including Electron menu bar)
- [ ] **Keyboard shortcut** — new or changed
- [ ] **CLI / env var** — new `od` subcommand or flag, new `tools-dev` / `tools-pack` / `tools-pr` flag, or new `OD_*` env var
- [ ] **API / contract** — new `/api/*` endpoint, new SSE event, or changed shape in `packages/contracts`
- [ ] **Extension point** — new entry under `skills/`, `design-systems/`, `design-templates/`, or `craft/`, or change to the skills protocol
- [ ] **i18n keys** — added new translation keys (see `TRANSLATIONS.md` for the locale workflow)
- [ ] **New top-level dependency** — adding any new entry to the **root** `package.json` (`dependencies` or `devDependencies`); workspace-package `package.json` files are out of scope. Include a paragraph on what we get vs. what bytes we ship (see `CONTRIBUTING.md` → Code style)
- [ ] **Default behavior change** — changes what existing users experience without opting in (default model, default setting, file/SQLite schema, auto-network on startup, auto-install)
- [ ] **None** — internal refactor, docs, tests, or translation update only
## Screenshots
<!-- If you checked "UI" above, attach screenshots showing the entry point —
where users discover the change — not just the feature in isolation.
Before/after is best for behavior changes. Short GIFs welcome. -->
## Bug fix verification
<!-- Skip if this PR isn't a bug fix.
Per AGENTS.md → "Bug follow-up workflow", bugs should be encoded as a
falsifiable test that goes red before the source change. Confirm:
- Test path that reproduces the bug:
- Did the test go red on `main` and green on this branch? (yes / no)
- If a red spec wasn't cheap to write, explain why and what verification
you did instead. -->
- -
## Validation ## Validation
<!-- What you actually ran. Default minimum: `pnpm guard` + `pnpm typecheck`,
plus the package-scoped tests/build for the files you changed
(e.g. `pnpm --filter @open-design/web test`). -->
- -

View file

@ -71,6 +71,16 @@ This file is the single source of truth for agents entering this repository. Rea
- Git commits must not include `Co-authored-by` trailers or any other co-author metadata. - Git commits must not include `Co-authored-by` trailers or any other co-author metadata.
## Pull request expectations
- Opening a PR uses `.github/pull_request_template.md`; fill every section, not just the title.
- "Why" must answer both the author's use case (what made you write this PR) and the pain being addressed (user problem, technical debt, prod issue, or unblocker), not just a one-line restatement of the title.
- "What users will see" describes the change from a user's perspective — what they click, what new thing appears, what default behavior changed — not from a code perspective.
- The Surface area checklist must reflect actual surfaces touched; check every box that applies, including extension points (`skills/`, `design-systems/`, `design-templates/`, `craft/`), CLI flags, env vars, i18n keys, and new root `package.json` dependencies.
- If any UI surface is checked, attach screenshots showing the entry point — where users discover the change — not just the feature in isolation; before/after is best for behavior changes.
- For bug-fix PRs, link the red-spec test that reproduces the bug and confirm it went red on `main` and green on the branch, per the `Bug follow-up workflow` section above.
- `CONTRIBUTING.md` covers PR scope, title format, dependency policy, and the issue-first rule for non-trivial features; `docs/code-review-guidelines.md` is the reviewer-facing complement.
## Code review guide ## Code review guide
- Use `docs/code-review-guidelines.md` as the repository-wide review standard. That document is the operational guide; this `AGENTS.md` is the source of truth when the two disagree. - Use `docs/code-review-guidelines.md` as the repository-wide review standard. That document is the operational guide; this `AGENTS.md` is the source of truth when the two disagree.

View file

@ -245,6 +245,7 @@ Beyond that:
- **One concern per PR.** Adding a skill + refactoring the parser + bumping a dep is three PRs. - **One concern per PR.** Adding a skill + refactoring the parser + bumping a dep is three PRs.
- **Title is imperative + scope.** `add dating-web skill`, `fix daemon SSE backpressure when CLI hangs`, `docs: clarify .od layout`. - **Title is imperative + scope.** `add dating-web skill`, `fix daemon SSE backpressure when CLI hangs`, `docs: clarify .od layout`.
- **Use the PR template.** Fill every section of [`.github/pull_request_template.md`](.github/pull_request_template.md) — Why, What users will see, Surface area, Screenshots (if UI), Bug fix verification (if bug fix), Validation. Empty sections earn a "please fill in" reply.
- **Body explains the why.** "What does this do" is usually obvious from the diff; "why does this need to exist" rarely is. - **Body explains the why.** "What does this do" is usually obvious from the diff; "why does this need to exist" rarely is.
- **Reference an issue** if there is one. If there isn't and the PR is non-trivial, open one first so we can agree the change is wanted before you spend the time. - **Reference an issue** if there is one. If there isn't and the PR is non-trivial, open one first so we can agree the change is wanted before you spend the time.
- **No squash-during-review.** Push fixups; we'll squash on merge. - **No squash-during-review.** Push fixups; we'll squash on merge.