120 lines
8.3 KiB
Markdown
120 lines
8.3 KiB
Markdown
---
|
||
name: create-pr
|
||
description: Write pull request titles and descriptions when creating a PR. Use when running `gh pr create`, opening a pull request, drafting a PR body or title, or when the user asks for help writing a PR description. Produces a drafted title and body which are then passed to `gh pr create`. Encodes principles from Google eng-practices, the Linux kernel's SubmittingPatches, and Chris Beams' commit-message guide, and prevents the AI-generated-PR anti-patterns (hype tone, unsubstantiated claims, diff-restatement).
|
||
license: Apache-2.0
|
||
metadata:
|
||
author: johnstegeman
|
||
version: "1.0"
|
||
---
|
||
|
||
# Writing pull request descriptions
|
||
|
||
You are about to create a PR. Your job is to produce a title and body that tell the reviewer **what the diff cannot**: why this change exists, what mental model they need to read it, and how you know it works.
|
||
|
||
## Workflow
|
||
|
||
Run these steps in order. Steps 1 and 2 can run in parallel.
|
||
|
||
### 1. Discover repo conventions
|
||
Read whichever of these exist (skip silently if missing):
|
||
- `.github/PULL_REQUEST_TEMPLATE.md` — if present, treat its sections as the skeleton
|
||
- `CONTRIBUTING.md`, `AGENTS.md`, `CLAUDE.md` — pull any documented PR conventions
|
||
- Last 3 merged PRs via `gh pr list --state merged --limit 3 --json title,body` — mirror the title style and length norms you see
|
||
|
||
If a `PULL_REQUEST_TEMPLATE.md` exists, use it as the structural skeleton. Don't invent a different one.
|
||
|
||
### 2. Gather change context
|
||
- `git log <base>..HEAD --oneline` — full commit history for the branch (not just the latest commit)
|
||
- `git diff <base>...HEAD --stat` — scope of changes
|
||
- Branch name — often encodes issue number (e.g. `jstegeman/ENG-1234-fix-x`)
|
||
- Scan commits and branch name for issue references (`#123`, `ENG-1234`, `JIRA-5`)
|
||
|
||
### 3. Draft the title
|
||
Rules in priority order:
|
||
1. **If the repo's recent PRs follow a convention, mirror it.** Conventional Commits adoption is genuinely contested — don't impose it where it isn't already in use.
|
||
2. **Imperative mood**: "Add retry logic" not "Added retry logic" or "Adds retry logic". Test: *"If applied, this commit will _[title]_"*.
|
||
3. **≤ 70 characters.** Detail goes in the body.
|
||
4. **Capitalize, no trailing period.**
|
||
5. **Specific over generic.** "Fix bug" / "Update code" / "Refactor" are banned — they fail the standalone-skim test.
|
||
6. If the repo uses Conventional Commits, prefix with `feat:`, `fix:`, `chore:`, `docs:`, `refactor:`, `perf:`, `test:`, `ci:`, `build:`, or `style:`. Append `!` for breaking changes.
|
||
|
||
### 4. Draft the body — adaptive to PR size
|
||
|
||
**Trivial PR** (typo, comment fix, one-line config tweak): one sentence stating *why* is enough. Skip headers.
|
||
|
||
**Typical PR**: cover these three, in this order, no rigid headers required:
|
||
- **Why** — the problem or motivation. What's broken, what's missing, what triggered this. The reviewer can read the diff to see *what* changed; they need you to explain *why*.
|
||
- **What changed at a high altitude** — the mental model the reviewer needs. Not a token-level restatement of the diff. ("Reworks the retry loop to back off exponentially" — not "Adds variable `delay`, multiplies by 2 in `foo()`.")
|
||
- **How you verified it** — tests added, manual steps run, commands executed, output observed. If you didn't test something, say so explicitly.
|
||
|
||
**Large or risky PR**: add as needed:
|
||
- **Breaking changes / API surface deltas** — explicit callout if public contracts changed
|
||
- **Out of scope / follow-ups** — what you intentionally did NOT do, so reviewers don't ask
|
||
- **Alternatives considered** — when a non-obvious design choice was made
|
||
|
||
**Always include if applicable**:
|
||
- Linked issue with auto-close keyword: `Fixes #123`, `Closes ENG-1234`. Auto-detect from branch name or commits.
|
||
|
||
### 5. Self-check before output
|
||
Before passing to `gh pr create`, re-read your draft against this checklist:
|
||
|
||
- [ ] Does the title pass the **standalone-skim test**? (Could someone scanning `git log --oneline` understand what this PR does?)
|
||
- [ ] Does the body explain **why**, not just **what**?
|
||
- [ ] Have I avoided **restating the diff in prose**?
|
||
- [ ] Are any **claims I made backed by evidence** I can point to in the PR?
|
||
- [ ] Have I cut every **hype word** (see list below)?
|
||
- [ ] If the description is getting long, **is the PR actually too big**? (Kernel rule: long description often = wrong-sized patch.)
|
||
|
||
### 6. Execute
|
||
Pass the drafted title and body to `gh pr create` per the harness's normal PR-creation flow.
|
||
|
||
## Core principles
|
||
|
||
1. **Why over what** (Beams rule 7; Linux kernel). Code shows what; the description must explain why. The Linux kernel: *"Whether your patch is a one-line bug fix or 5000 lines of a new feature, there must be an underlying problem that motivated you to do this work."*
|
||
|
||
2. **Self-contained** (Linux kernel). A reviewer or future archaeologist should understand the change from the title + body without chasing external context. Issue links are aids, not substitutes.
|
||
|
||
3. **Standalone-skim test** (Google eng-practices). The title alone must let someone skimming history understand what this CL does. Banned title patterns from Google: `Fix bug`, `Fix build`, `Add patch`, `Moving code from A to B`.
|
||
|
||
4. **Quantify or hedge** (Linux kernel). *"If you claim improvements in performance, memory consumption, stack footprint, or binary size, include numbers that back them up. But also describe non-obvious costs."* If you don't have numbers, label the claim `(not measured)` or remove it — don't assert it confidently.
|
||
|
||
5. **Long description = wrong-sized PR** (Linux kernel). *"If your description starts to get long, that's a sign that you probably need to split up your patch."* If you find yourself writing four paragraphs of context, consider whether the PR should be split.
|
||
|
||
## Anti-patterns — these break the skill
|
||
|
||
**Restating the diff in prose.** "Added function `processOrder` that calls `validateOrder` and then `submitOrder`." The reviewer can read code. Tell them the *shape* of the change, not its tokens.
|
||
|
||
**Marketing / hype language.** Cut these words on sight — they signal AI-generated content and add no information:
|
||
|
||
> comprehensive, robust, seamlessly, leverages, leverage, ensures, enables, empowers, streamlines, elegant, clean, intuitive, powerful, sophisticated, cutting-edge, modern, optimal, best-in-class, production-ready, enterprise-grade
|
||
|
||
Also cut decorative emoji in headers, marketing-style intros ("This PR brings exciting improvements to…"), and self-congratulatory closes ("This sets us up for future success.").
|
||
|
||
**Unsubstantiated claims.** "Improves performance" with no number. "More reliable" with no failure mode named. "Better UX" with no before/after. Either name the evidence or rewrite the claim honestly: *"Should reduce contention in `handleX` based on the lock-ordering change; not measured."*
|
||
|
||
**Walls of text on tiny PRs.** A one-line typo fix does not need six headers. Match structure to scope.
|
||
|
||
**Over-confident testing claims.** "Tested" alone is meaningless. Name *what* you tested: command run, test file touched, manual repro reproduced. If you didn't test a code path, say so.
|
||
|
||
**Restating what's already in the linked issue.** Link the issue; don't copy-paste its body into the PR description. The reviewer can click.
|
||
|
||
## Example: typical PR body
|
||
|
||
> The retry middleware was retrying on 4xx responses, which amplified load against an already-failing dependency during the Tuesday incident.
|
||
>
|
||
> This narrows retries to 5xx, network errors, and explicit `Retry-After` responses. Other 4xx propagate to the caller unchanged.
|
||
>
|
||
> Verified by adding `retry_middleware_test.py::test_no_retry_on_4xx` and re-running the failure-mode replay in `scripts/replay.py` against the recorded incident traffic — request count drops from 6.2× to 1.0× as expected.
|
||
>
|
||
> Fixes ENG-4421.
|
||
|
||
Notice: no headers, no hype, one number where a number exists, evidence named, issue linked. ~90 words. The diff fills in the rest.
|
||
|
||
## Primary sources
|
||
|
||
- Google eng-practices, *CL descriptions*: https://google.github.io/eng-practices/review/developer/cl-descriptions.html
|
||
- Linux kernel, *Submitting patches*: https://www.kernel.org/doc/html/latest/process/submitting-patches.html
|
||
- Chris Beams, *How to Write a Git Commit Message*: https://cbea.ms/git-commit/
|
||
- Tim Pope, *A Note About Git Commit Messages*: https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
|
||
- Conventional Commits 1.0: https://www.conventionalcommits.org/en/v1.0.0/
|