Back to Details

writing-pull-requests

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

Writing PR/MR bodies

撰写PR/MR正文

The job of a PR/MR description is to compress what the reviewer needs to know that is not visible in the diff. Apply this test to every sentence.
To embed screenshots or video, read MEDIA.md.
PR/MR描述的作用是提炼出评审者需要了解但无法在代码差异（diff）中看到的信息。每一句话都要经过这个标准的检验。
如需嵌入截图或视频，请阅读MEDIA.md。

One block, one line

一个区块，一行内容

GitHub renders a single newline inside a paragraph as 
<br>
in issue, pull-request, and discussion fields (documented; release notes behave the same in practice), so a hard-wrapped paragraph displays broken mid-sentence. Write each paragraph or list item as one line, however long, with a blank line between blocks — this renders correctly on every forge, including GitLab, whose fields follow standard Markdown paragraph rules. Markdown files in the repo (README, docs, 
CHANGELOG.md
) render paragraphs normally; the rule applies the moment their content is pasted into a comment-style field — a changelog entry pasted into release notes is the common trap. Commit messages stay wrapped at 72 columns.
GitHub会将段落内的单个换行符渲染为
<br>
，这一规则适用于议题、拉取请求和讨论字段（已有官方文档说明；发布说明在实际使用中也遵循相同规则），因此硬换行的段落会在句子中间显示断裂。请将每个段落或列表项写成一行（无论长度如何），区块之间留空行——这种写法在所有代码托管平台（包括GitLab，其字段遵循标准Markdown段落规则）上都能正确渲染。仓库中的Markdown文件（README、文档、
CHANGELOG.md
）会正常渲染段落；但当内容被粘贴到评论类字段时，该规则就适用了——常见的陷阱是将变更日志条目粘贴到发布说明中。提交信息仍需保持72列换行。

The necessity test

必要性检验

Before writing a sentence, ask: can the reviewer get this from a 30-second scan of the diff? If yes, cut it.
Cut, because the diff already says this:
  • "Adds a 
    Record
    dataclass with six fields: a, b, c, ..." (read the class definition).
  • "Updated all callers from dict access to attribute access." (read any hunk).
  • "Function signatures changed from 
    dict
    to 
    Record
    ." (read the signature).
  • "No behavior change intended." (the 
    refactor:
    type label and absent test changes already say this).
Keep, because the reviewer can't get this from a diff scan:
  • "Stacked on #200. Merge that first."
  • "
    timestamp = None
    is the sentinel; the existing fallback path was already designed for it, so semantics unchanged."
  • "Safe because the indexer only reads 
    Base
    fields."
在写每一句话之前，先问自己：*评审者通过30秒的diff扫描就能获取这个信息吗？*如果答案是肯定的，就删掉这句话。
以下内容可以删掉，因为diff已经展示了：
  • "添加了包含六个字段的
    Record
    数据类：a, b, c, ..."（查看类定义即可）。
  • "将所有调用者从字典访问更新为属性访问。"（查看任意代码块即可）。
  • "函数签名从
    dict
    改为
    Record
    。"（查看函数签名即可）。
  • "无意改变任何行为。"（
    refactor:
    类型标签和未修改测试的情况已经说明了这一点）。
以下内容需要保留，因为评审者无法通过diff扫描获取：
  • "基于#200堆叠提交，请先合并该PR。"
  • "
    timestamp = None
    是标记值；现有回退路径已为此设计，因此语义未发生变化。"
  • "此操作安全，因为索引器仅读取
    Base
    字段。"

Match length to change weight

描述长度匹配变更重要性

A mechanical or conventional change (rename sweep, formatting, dependency bump, dead-code removal) needs only a few sentences: the rationale, the "no logic change" claim, and any exceptions summarized as a category, not enumerated per file. Name the sweep and its pattern once — "Mechanical rename sweep across 40 files: key access becomes attribute access; no logic change." — and let the diff carry every instance. If the reviewer can verify the whole diff by pattern-matching one hunk, the description should be readable in one breath. Reserve multi-paragraph descriptions and itemized non-obvious-bits lists for changes whose hunks genuinely differ from each other.
机械性或常规变更（批量重命名、格式调整、依赖版本升级、死代码移除）只需要几句话：变更理由、“无逻辑变更”声明，以及任何例外情况的分类总结（无需逐个文件列举）。只需说明一次批量操作及其模式——例如“对40个文件进行机械性批量重命名：键访问改为属性访问；无逻辑变更。”——具体实例由diff展示。如果评审者只需匹配一个代码块的模式就能验证整个diff，那么描述应该能一口气读完。只有当各个代码块内容差异较大的变更，才需要使用多段落描述和列出非显而易见的细节。

Lead with the punchline

开门见山，直击核心

The first sentence answers why does this exist?, not what does this do?. The "what" is the diff.
WeakStrong
"This MR migrates 
record
from dict to dataclass."		"Finishes what #200 started."
"Refactors the auth module.""Replaces the layered try/except in auth with a dispatch table; the old structure swallowed CSRF errors on token refresh."
"Adds a new endpoint.""Third try at 
/api/v2/widgets
; v1 returned the wrong cache key, the spike PR ran into rate limits."
If you can't write a strong lead, you may not understand the change well enough to describe it. Re-read the diff.
第一句话要回答为什么要有这个变更？，而不是这个变更做了什么？。“做了什么”已经由diff展示。
欠佳表述优秀表述
"此MR将
record
从字典迁移为数据类。"		"完成#200未完成的工作。"
"重构认证模块。""用调度表替换认证模块中分层的try/except结构；旧结构会在令牌刷新时掩盖CSRF错误。"
"添加一个新的端点。""第三次尝试实现
/api/v2/widgets
；v1返回了错误的缓存键，原型PR遇到了速率限制问题。"
如果你写不出优秀的开篇句，可能你对变更的理解还不够深入。请重新阅读diff。

Surface non-obvious bits explicitly, each with safety resolution

明确列出非显而易见的细节，并附上安全性说明

When the diff has subtle decisions (sentinels, ordering swaps, intentional breaks), enumerate them. Each one ends with a safety claim so the reviewer doesn't have to come back with "but what about...".
Two non-obvious bits:

1. `timestamp = None` is the sentinel for "not yet parsed". The existing none-fallback was already doing this on dicts; nothing changes semantically.

2. The pipeline reorders normalization to run before indexing. The indexer only reads `Base` fields; safe.
The "; safe" / "; nothing changes" tail matters. Without it the reviewer has to verify the safety claim themselves.
当diff中包含微妙的决策（标记值、顺序调换、有意的中断）时，请逐一列举。每个细节都要附上安全性声明，这样评审者就不必回来追问“但是……怎么办？”
Two non-obvious bits:

1. `timestamp = None` is the sentinel for "not yet parsed". The existing none-fallback was already doing this on dicts; nothing changes semantically.

2. The pipeline reorders normalization to run before indexing. The indexer only reads `Base` fields; safe.
末尾的“; safe” / “; nothing changes”非常重要。如果没有这些内容，评审者就必须自行验证安全性声明。

What to cut

需要删除的内容

Recap of context the reviewer already has:
  • "This MR builds on #1234 which did X. That work left Y. This MR finishes Y." → "Finishes #1234."
  • "The original code did X. This changes it to Y." — before/after framing; the diff shows X and Y.
  • Restating code comments — the description is ephemeral; the comment persists with the code.
Apologies and framings:
  • "This is a large change but..."
  • "Please review carefully because..."
  • "Sorry for the noise in file Z..."
Templates without signal:
  • A 
    ## Summary
    header with one bullet that is already the lead sentence.
  • A 
    ## Why
    header on a docs typo fix.
  • - [x] make lint
    in a docs-only test plan.
Marketing words: "headline", "elegant", "robust", "sells", "deliberately", "carefully crafted".
评审者已知的上下文回顾：
  • "此MR基于#1234，该PR完成了X。那项工作留下了Y，此MR完成Y。" → "完成#1234的工作。"
  • "原代码做了X，现在改为Y。" ——这种前后对比的表述；diff已经展示了X和Y。
  • 重复代码注释——描述是临时的，而注释会随代码永久保留。
道歉和铺垫表述：
  • "这是一个较大的变更，但是……"
  • "请仔细评审，因为……"
  • "抱歉文件Z里有冗余内容……"
无实际信息的模板内容：
  • ## 摘要
    标题下只有一个项目符号，而该内容已经是开篇句。
  • 文档笔误修复中出现
    ## 原因
    标题。
  • 纯文档变更的测试计划中出现
    - [x] make lint
    。
营销类词汇： "headline"、"elegant"、"robust"、"sells"、"deliberately"、"carefully crafted"。

Worked example

示例演示

A 
dict
→ dataclass migration MR. Bloated draft (~190 words):
Eliminates the dict / dataclass boundary inside the pipeline. 
Container.items
was already typed 
list[Record]
after #200 only because the items happened to be dicts at runtime; now they actually are Record instances with attribute access throughout.
Record is a frozen kw_only dataclass with six required fields: a, b, c, d, e, f. The conversion from the parser's dict output happens once at its boundary inside 
_normalize_items
; 
timestamp = None
is the "unknown" sentinel that the existing fall-back-to-
now()
path was already designed for. The parser still takes dicts, so a small bridge round-trips items via 
dataclasses.replace
.
The pipeline is reordered so the second normalization call runs on 
ContainerBase
before promoting to 
Container
. ...
[...3 more bullets recapping what the diff shows...]
After the necessity test (~75 words):
Finishes what #200 started. 
record
is now actually a 
Record
instance, not a dict that happened to flow through a field typed 
list[Record]
.
Two non-obvious bits:
  1. timestamp = None
    is the sentinel for "not yet parsed". The existing none-fallback was already doing this on dicts; nothing changes semantically.
  2. The pipeline reorders the second normalization call to run on 
    ContainerBase
    before promoting to 
    Container
    . The function only reads 
    ContainerBase
    fields; safe.
Stacked on #200.
一个字典→数据类迁移的MR描述。冗长的初稿（约190词）：
消除了流水线内部字典/数据类的边界。
Container.items
在#200之后被标注为
list[Record]
类型，但实际上运行时仍是字典；现在它们在整个流程中都是真正的Record实例，使用属性访问。
Record是一个冻结的kw_only数据类，包含六个必填字段：a, b, c, d, e, f。从解析器的字典输出转换为Record实例的操作在
_normalize_items
的边界处执行一次；
timestamp = None
是“未知”标记值，现有回退到
now()
的路径已为此设计。解析器仍然接受字典，因此通过
dataclasses.replace
实现了一个小型的双向转换桥接。
流水线的顺序被调整，第二次归一化调用在提升为
Container
之前先在
ContainerBase
上运行。……
[...另外3个项目符号重复了diff中已展示的内容...]
经过必要性检验后的版本（约75词）：
完成#200未完成的工作。
record
现在是真正的
Record
实例，不再是碰巧流经
list[Record]
类型字段的字典。
两个非显而易见的细节：
  1. timestamp = None
    是“尚未解析”的标记值。现有空值回退路径在字典上已经实现了该逻辑；语义未发生变化。
  2. 流水线调整了第二次归一化调用的顺序，使其在提升为
    Container
    之前先在
    ContainerBase
    上运行。该函数仅读取
    ContainerBase
    字段；操作安全。
基于#200堆叠提交。