PR Review English — GitHub PR 描述、评论、回评的英语

读完本页后,你应该能够:

• 写一个 reviewer 能 30 秒看懂 "what / why / how to verify" 的 PR description
• 用 4 档 review comment 强度(nit / suggestion / blocker / question)对应不同情况
• 礼貌但坚定地 request changes,不让对方下不来台
• 用 GitHub-specific 语法(suggestion block / code reference / co-author)
• 回别人 review 时用 acknowledge → action → reference 三步,不防御
• 识别中国工程师 5 种 Chinglish PR 反模式

reviewer 30 秒决定要不要花时间。前 3 行决定一切。

1.1 标准模板

## What

[one-sentence summary of what this PR changes]

## Why

[why this change is needed — link to issue / RFC / bug / customer ask]

## How

[high-level approach — algorithm choice / architecture / why this over alternatives]

## How to test

- [ ] Unit tests: `pnpm test src/foo`
- [ ] Manual: open /admin → click X → expect Y
- [ ] CI: green ✓

## Screenshots / Recording

[if UI change]

## Notes

[anything that won't be obvious from the diff: deprecations / migrations / 
follow-ups / known limitations]

1.2 标题规范

✓ feat(auth): rate-limit login attempts per IP
✓ fix(api): handle null response from billing service
✓ refactor(db): extract retry logic to shared util
✓ docs(readme): clarify dev setup for M1 macs

陷阱(中国 PR 常见):

✗ Update auth.ts                        ← 太 vague
✗ Fix bug                               ← 哪个 bug
✗ As discussed in the meeting yesterday ← 别人不在那个 meeting
✓ fix(auth): allow special chars in password (#1234)

1.3 What/Why/How 三段每段的写法

What —— 一句话,过去时(动作完成),具体动词:

✓ Adds retry-with-backoff to the billing webhook handler.
✓ Removes the deprecated /v1/users endpoint.
✓ Migrates the cache from in-memory to Redis.

Why —— 必须带链接,说原因:

✓ Closes #1234. Customer reports webhook timeouts during peak traffic;
  current handler has no retry, so transient billing-service hiccups
  result in lost notifications.

✓ Per RFC #56, we're consolidating cache layers to one Redis instance
  to simplify the deployment topology.

不要写空话:

✗ This PR improves the system.            ← 改善什么?
✗ We need this for the next release.       ← 为啥

How —— 算法 / 架构 / 为什么这种选择:

✓ Uses exponential backoff (1s → 2s → 4s, max 3 retries) with jitter
  per AWS Architecture Blog recommendation. Considered linear backoff
  but exponential is standard for downstream service protection.

1.4 反模式 PR description

✗ "Fixed it"
✗ "As discussed"
✗ "Small refactor"
✗ "WIP, don't review yet"     ← 那为啥开 PR 不开 draft
✗ Description: 1 行 + 100 file 的 diff

不是所有 comment 都同等重要。显式标记强度让作者快速分流:

2.1 4 档 prefix(社区约定)

部分团队用 must / should / could 三档。本质一样:让作者知道哪条必改 / 哪条仅供参考。

2.2 各档典型句型

nit:

nit: trailing whitespace on line 47
nit: would prefer `userId` over `uid` for consistency with the rest of the file
nit: missing period at end of sentence

suggestion:

Suggestion: this loop could be a `.map()` for readability. Up to you.
Could you extract the validation logic to a separate function? The current
function is doing two things.
Have you considered using Set instead of Array.includes() here? Should be
O(1) vs O(n).

question:

Question: is there a reason to retry on 4xx? My intuition says we should
only retry 5xx and timeout.
Wait, where does `MAX_RETRIES` come from? I don't see it imported.
What happens if the user is offline when this fires?

blocker:

[blocker] This will break the auth tests — `currentUser` can be null in
the unauth path (see test/auth.test.ts:42).
[blocker] Memory leak: the listener registered on line 87 is never
removed in cleanup. We saw this exact issue in #889.
[blocker] Security: this allows path traversal via `../../`. We need to
sanitize `filename` before passing to fs.readFile.

2.3 礼貌的 request-change chunks

即使是 blocker,也用 chunks 缓冲:

"I think there's a subtle issue here..."
"One concern: ..."
"Could we [alternative]? Reasoning: ..."
"This might be a bug — could you double-check by [test]?"

避免 directly hostile:

✗ "This is wrong."
✗ "Why would you do this?"
✗ "This won't work."

改成:

✓ "I think this might not handle the [X] case — could you check?"
✓ "Could you walk me through the reasoning here? I'm missing something."
✓ "I don't think this approach scales past N users — see [reference]."

2.4 给 positive feedback 也要

reviewer 不只是找 bug。看到好的也评:

"Nice catch on the race condition!"
"This is a much cleaner abstraction than what was there before."
"Love the test coverage here."
"I learned something — didn't know about [feature]."

让 PR 不全是负面。

回 PR comment 比回邮件更短,但仍要承认 → action → reference 三步:

3.1 接受 + 改了

"Good catch, fixed in [latest commit](link)."
"Done in cd34abc — also added a test."
"Updated, thanks!"
"You're right — extracted to a helper. PTAL."   (PTAL = please take another look)

3.2 部分同意

"Good point on naming — renamed. On the second thing, I think the
current behavior is intentional because [reason] — happy to discuss."

"Agreed on the cleanup, but I'd prefer to do it in a follow-up PR to
keep this one focused. Filed as #5678."

3.3 不同意

"I see your concern, but I think the current approach is fine because
[reason 1, 2]. Open to changing if you feel strongly."

"Hmm, I considered that, but [why not]. Thoughts?"

"I'd push back gently — [alternative argument]. But not a hill I'd die
on; let me know."

3.4 没看懂 reviewer 想说什么

"Could you say more about what you mean? I'm not sure I'm following."
"Sorry, can you give me a concrete example of where this breaks?"
"Is the concern X or Y?"

3.5 标记 resolved

GitHub 有 "Resolve conversation" 按钮。约定:作者 fix 后 reviewer 标 resolved(不是作者自己),除非作者完全 ack 了 nit。

4.1 Suggestion blocks(reviewer 给具体改法)

```suggestion
const userId = req.user?.id ?? null;
```

作者点 "Apply suggestion" 直接 commit。特别适合 nit / 简单 fix,省作者打字。

4.2 Code reference(链接到代码)

This conflicts with [the assumption in
auth/middleware.ts#L42-L48](URL).

GitHub 自动渲染成 inline code preview。

4.3 Co-author commit

PR 多人协作时:

Co-authored-by: Alice Liu <alice@example.com>

放 commit message 末尾,GitHub 把 ta 算入 contributor。

4.4 Closes / Fixes / Resolves

PR description 写 Closes #1234 / Fixes #1234 —— merge 时 GitHub 自动关 issue。

Closes #1234, #1235.
Fixes part of #1236 (still need to handle the Edge browser case).

4.5 Mention 召唤

@alice could you take a look at the auth changes?
cc @security-team for awareness on the ACL change

@team 召唤整个团队;cc @user 不算 review 请求,只是知会。

实际 PR description + 一轮典型 review + 回评:

fix(billing): retry webhook on transient 5xx with exp backoff (#1234)

## What

Adds exponential backoff retry to the billing-service webhook handler.

## Why

Closes #1234. Customer reports lost webhooks during peak traffic. The
current handler returns immediately on any error, so a transient 503
from billing-service results in dropped state updates. Retry with
backoff converts these into eventual successes.

## How

- Wraps `processBillingEvent()` in a `withRetry()` helper.
- Exponential backoff: 1s → 2s → 4s, max 3 retries (matches AWS guidance).
- Retries only on 5xx and network errors. 4xx is treated as fatal
  (request was malformed, retry won't help).
- Adds jitter (±25%) to avoid thundering herd if multiple events fail
  at the same instant.

## How to test

- [x] New unit tests in `billing.test.ts` (mock 503 → success on retry 2)
- [x] Manual: triggered fake 503 in staging — saw 3 retry attempts in
      logs, then success
- [x] CI green

## Notes

- Did not retry 4xx because customer's payload bug shouldn't loop forever.
  If we ever need it, easy to add a list of retryable 4xx codes.
- Follow-up: dashboard for retry rate per endpoint (#1245).

Reviewer comment:

[blocker] What if `withRetry()` itself throws (e.g., calling code is
in a request handler that times out at 5s, but our retry can take
1+2+4 = 7s)? We'd silently lose the request without retry.

Could you add a max-total-time guard, or document that the caller is
responsible for setting a wider timeout?

作者回:

Good catch, fixed in cd34abc:
- Added `maxTotalMs` param (default 6000ms)
- If exceeded, fail fast with a `RetryTimeoutError` (so caller can
  distinguish from final-attempt failure)
- Updated tests to cover the timeout case

Also added a JSDoc note about the caller's responsibility to set a
wider timeout. PTAL.

逐条:

• Reviewer 用 [blocker] 标记必改
• 用 question chunks "What if X" 不直接说 "this is broken"
• 给 alternative ("max-total-time guard") + 让作者选
• 作者 acknowledge ("Good catch") + 4 行说改了什么 + commit hash + PTAL closer

7.1 开始 review

"Starting the review now."
"Will get to this today."
"Reviewing — give me 15 min."

7.2 一遍 review 完(全 approve)

"LGTM!"  ✓
"Approved — ship it."
"This looks clean. Approved."
"LGTM with one nit (non-blocking)."

7.3 Request changes

"Mostly LGTM, a few questions inline."
"Looks good overall — couple of blockers to address before merge."
"I have some concerns about [aspect] — see comments."

7.4 Approve with conditions

"Approving — fix the type error and ship."
"LGTM after the test fix."
"OK to merge once CI is green."

7.5 把 review 转给别人

"This is more in @alice's area — could you take this one?"
"I don't have context on the auth subsystem; deferring to @bob."

7.6 长 review 截止

"Out of time today, will continue tomorrow."
"Got through the first half — more comments coming."
"Need to think about this overnight before approving."

跟欧美同事时差 8-12 小时,async PR review 是标配:

"No rush — please review when you get a chance."
"Heads up: I'll be offline next 8 hours but can address comments
tomorrow morning Asia time."
"Will pick this up at start of my day (UTC+8 morning)."
"Apologies for the slow turnaround — was on holiday."
"This PR is blocking [thing] — could we get a quick review?"
"OK to merge for me; happy to address any post-merge comments in a
follow-up."

• PR description 是 reviewer 的 30 秒决定 —— 标题具体 + What/Why/How 三段 + How-to-test + Notes
• Review comment 用 4 档 prefix —— nit: / suggestion: / question: / [blocker] 让作者快速分流
• request-change 用 question chunks 缓冲 —— "I think this might not handle X" 不直接 "this is wrong"
• 回评用 acknowledge → action → reference 三步 —— "Good catch, fixed in [hash]. PTAL."
• GitHub 语法用熟:Closes #1234 / suggestion blocks / code reference / @team mention / Co-authored-by
• Positive feedback 也要给 —— "Nice catch" / "Cleaner than before" / "Learned something"
• 跨时区 chunks:"no rush" / "heads up offline 8h" / "OK to merge for me" 让 async 协作不憋
• 5 反模式戒除:流水账 / 防御回评 / 太软 request / 太硬 nit / 空话致谢
• Resolve conversation 约定:reviewer 标,不是作者自己标(除非纯 nit)

• ← 索引
• Daily Writing Toolbox(时态/句型基础)
• Business Email(模板 + 跨时区)
• Tech Interview(STAR + clarifying)
• Peer Review Response(R&R letter)
• Engineering Writing(paper / report)
• Meeting & Presentation chunks
• Tech Collocations