engineering-review-comments

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

Engineering Review Comments

工程评审评论

Review comments should improve the code and the developer's future judgment. They should be clear about severity, grounded in engineering reasoning, and directed at the code rather than the person.
Adapted from Google Engineering Practices Documentation, especially "How to write code review comments" and "Handling pushback in code reviews." Source: https://google.github.io/eng-practices/review/reviewer/comments.html. License: CC-BY 3.0.
评审评论应能改进代码并提升开发者未来的判断能力。评论需明确严重性,基于工程逻辑,且针对代码而非个人。
改编自谷歌工程实践文档,尤其是《如何撰写代码评审评论》和《处理代码评审中的反驳》。来源:https://google.github.io/eng-practices/review/reviewer/comments.html。许可证:CC-BY 3.0。

Comment Shape

评论结构

Use this structure for non-trivial comments:
text
SEVERITY: Observation about the code.

Why this matters: engineering reason, user impact, maintainability impact, or violated project rule.

Requested action: concrete fix, question, or decision the author needs to make.
Keep tiny comments tiny. Use the full shape when intent could be misunderstood.
对于非琐碎的评论,请使用以下结构:
text
SEVERITY: 关于代码的观察结论。

重要性说明:工程层面的原因、对用户的影响、对可维护性的影响,或是违反的项目规则。

要求操作:作者需要执行的具体修复、需回答的问题,或需做出的决策。
简短评论保持简洁。当意图可能被误解时,使用完整结构。

Severity Labels

严重性标签

  • BLOCKER
    : must change before approval because correctness, maintainability, security, data, test, or operational risk is unacceptable.
  • IMPORTANT
    : should change before approval; can be waived only with an explicit rationale.
  • QUESTION
    : reviewer needs information before deciding.
  • NIT
    : minor polish. Do not block approval.
  • OPTIONAL
    or
    CONSIDER
    : a suggestion the author can decline.
  • FYI
    : educational context or future consideration; no action expected.
  • BLOCKER
    : 必须在批准前修改,因为正确性、可维护性、安全性、数据、测试或运营风险不可接受。
  • IMPORTANT
    : 应在批准前修改;仅在有明确理由时可豁免。
  • QUESTION
    : 评审者需要获取信息才能做出决定。
  • NIT
    : 细微的优化。不得阻碍批准。
  • OPTIONAL
    CONSIDER
    : 作者可拒绝的建议。
  • FYI
    : 教育性内容或未来参考;无需采取行动。

Writing Rules

撰写规则

  • Comment on the code, design, tests, or behavior; do not characterize the author.
  • Explain why when asking for anything beyond obvious style or mechanical cleanup.
  • Prefer "this makes X harder because..." over "I don't like X."
  • Ask for simpler code when a review-thread explanation is needed to understand ordinary behavior.
  • Accept a code comment only when the code cannot reasonably express the why by itself.
  • Provide direct code or a specific design only when it is faster or safer than making the author infer it.
  • Otherwise describe the problem and let the author choose the implementation.
  • Point out good engineering choices when useful, and say why they are good.
  • 针对代码、设计、测试或行为发表评论;不要针对作者本人做出评价。
  • 当要求进行超出明显风格或机械性清理的修改时,需说明原因。
  • 优先使用“这会导致X更难实现,因为……”而非“我不喜欢X”。
  • 当需要通过评审线程解释才能理解常规行为时,要求简化代码。
  • 仅当代码本身无法合理表达其背后原因时,才接受添加代码注释。
  • 仅当直接提供代码或特定设计比让作者自行推断更快或更安全时,才这么做。
  • 否则只需描述问题,让作者选择实现方式。
  • 适时指出优秀的工程决策,并说明其优点。

Wording Patterns

措辞范例

Use these patterns directly when helpful:
text
BLOCKER: This path can return success before the write is durable.

If the process crashes after this point, callers will believe the operation completed even though the data may be missing. Please either move the success response after the durable write or make the operation explicitly asynchronous and observable.
text
QUESTION: What guarantees that this callback cannot run concurrently?

If it can run in parallel, `pending` can be decremented twice from the same value. If the framework serializes this callback, please add a short comment at the boundary where that guarantee comes from.
text
NIT: This helper name reads as if it validates the payload, but it only normalizes whitespace. Could we rename it to `normalizePayloadText`?
text
OPTIONAL: Consider extracting this branch into a named helper. The current version is correct, but a helper would make the three policy cases easier to scan.
如有帮助,可直接使用以下范例:
text
BLOCKER: 该路径可能在写入持久化前返回成功。

如果进程在此之后崩溃,调用方会认为操作已完成,但数据可能丢失。请将成功响应移至持久化写入之后,或使操作显式为异步且可观察。
text
QUESTION: 是什么保证了这个回调不会并发执行?

如果它可以并行运行,`pending`可能会被从同一个值递减两次。如果框架会序列化这个回调,请在提供该保证的边界处添加简短注释。
text
NIT: 这个辅助函数的名称看起来像是验证负载,但它仅规范化了空白字符。我们可以将其重命名为`normalizePayloadText`吗?
text
OPTIONAL: 考虑将这个分支提取为一个命名辅助函数。当前版本是正确的,但辅助函数会让三种策略案例更易于浏览。

Handling Pushback

处理反驳

When the author disagrees:
  1. Re-evaluate their argument first. They may be closer to the code or have context you missed.
  2. If they are right, say so plainly and resolve the comment.
  3. If you still disagree, restate their point, then explain the code-health reason for your request.
  4. Distinguish mandatory requests from preferences.
  5. If the thread stops making progress, move to a higher-bandwidth discussion and summarize the decision back in the review.
Do not keep arguing because you wrote the first comment. The goal is the best code, not defending the initial wording.
当作者提出异议时:
  1. 先重新评估他们的论点。他们可能更熟悉代码,或掌握你遗漏的上下文。
  2. 如果他们是对的,直接说明并解决该评论。
  3. 如果你仍不同意,先重述他们的观点,然后解释你的要求背后的代码健康性原因。
  4. 区分强制性要求与个人偏好。
  5. 如果讨论无法取得进展,转向更高带宽的沟通方式,并将决策总结回评审中。
不要因为你先提出了评论就一直争论。目标是得到最优代码,而非捍卫最初的措辞。

Common Mistakes

常见错误

  • Leaving unlabeled comments that authors interpret as mandatory.
  • Writing a correct critique in a tone that makes collaboration harder.
  • Asking the author to explain confusing code only in the review tool.
  • Turning every comment into a complete design assignment.
  • Treating educational notes as blockers.
  • 留下未标记的评论,导致作者误认为是强制性要求。
  • 用会阻碍协作的语气撰写正确的批评。
  • 仅在评审工具中要求作者解释令人困惑的代码。
  • 将每条评论都变成完整的设计任务。
  • 将教育性笔记视为阻塞项。