engineering-review-feedback

工程评审反馈

将评审者的反馈视为共同改进代码的机会。最佳回应往往是修改代码、测试或注释，让未来的读者也能受益，而不是只在评审讨论串中解释。

改编自 Google Engineering Practices Documentation，尤其是“How to handle reviewer comments”。来源：https://google.github.io/eng-practices/review/developer/handling-comments.html。许可证：CC-BY 3.0。

回复流程

1. 如果评论让你感到沮丧，先暂停一下再回复。
2. 明确评审者提出的要求。
3. 对每条评论进行分类：
   • 需要修改代码。
   • 需要修改测试。
   • 需要修改文档或注释。
   • 需要澄清。
   • 存在分歧，需要讨论。
   • 非阻塞性建议。
4. 优先改进交付内容，而不是只在评审工具中解释。
5. 运行相关验证。
6. 回复说明改了什么、为什么改，以及如何测试。

如果评审者表示看不懂代码，先尝试让代码更清晰。只有当无法通过代码本身清楚表达原因时，才添加代码注释。

当你认同时

简短回复并做出修改。

Done. I renamed the helper to describe normalization rather than validation and updated the two call sites.

Tested with: npm test -- user-profile.test.ts

当你需要澄清时

询问缺失的决策或约束。避免使用防御性的措辞。

I want to make sure I understand the concern. Are you asking for this to reject duplicate requests at the API boundary, or for the worker to make duplicate processing idempotent?

当你不同意时

通过比较权衡来表达不同意见，而不是否定对方。

I chose X because it keeps the retry policy in one place and avoids a second queue state. My concern with Y is that it makes cancellation observable in two different layers.

If you think Y better serves the rollback requirement, I can switch to it. Is that the tradeoff you want prioritized?

如果评审者给出了更好的技术论据，就接受并调整。如果双方无法达成一致，就转到更适合深入沟通的渠道，并在评审中总结最终决定。

当反馈缺乏建设性时

不要以同样的方式回应。提取其中可以通过代码、测试或文档解决的技术问题。如果评论带有敌意、含糊不清或反复没有成效，就转到私下或更适合深入沟通的渠道，必要时请负责人介入，并仅在评审中总结技术决定。

在正确的位置修复问题

• 表达式令人困惑：简化表达式或重命名。
• 不明显的不变量：通过类型、验证、断言或针对性注释将其明确表达。
• 对行为缺乏信心：添加或改进测试。
• 缺少上下文：更新 PR 描述、代码注释、README 或设计文档。
• 评审者因过时的差异内容而产生误解：变基、删除无用代码或更新描述。
• 纯粹的个人偏好：询问是否属于必须要求；否则将其视为可选建议。

回复摘要模板

Addressed this round:
- Changed ...
- Added/updated tests for ...
- Clarified ...

Verification:
- `...`

Open:
- ...

常见错误

• 愤怒地回复或使用讽刺语气。
• 只在评审讨论串中解释令人困惑的代码。
• 不澄清建议的严重程度，就把评审者的所有建议都视为强制要求。
• 当前修改引入了复杂性，却说“以后再清理”。
• 处理范围很窄的评审意见时，顺手进行大范围且无关的重构。