代码审查 — 什么是代码审查，代码审查和PR检查如何工作

如何正确撰写审查评论

评论的语气——对代码审查的效率至关重要。"这是错误的"评论会引发防御性反应，不会给作者提供有用信息。最好的表述是问题-建议："你觉得这种方法怎么样？"，"如果user == nil，这里可能会发生NPE。也许添加一个guard？"。问题施加的压力较小，能激发讨论。

一个好的评论结构包括三个部分：什么错了，为什么是问题以及如何修复。例如："在这个循环中，由于嵌套的contains使用了O(n²)，在10k+记录时可能会变慢。尝试用Set替换以实现O(1)搜索"。这样的表述同时指出了问题，解释了其重要性并提供了解决方案——作者无需猜测。

GitHub和GitLab支持suggestions——内置的代码更改建议。审查者可以写："```suggestion 在处理前过滤空行```"——作者单击即可应用更改。这加速了小修改并减少了审查轮次。对于大修改，最好写一般评论，而不是在suggestion中放置大块内容。

# 好的代码审查评论模板

# 不好: “这段代码是错误的”
# 好: “我们可能会在空响应时丢失数据。
#         If response.data == nil, the guard returns nil,
#         and user sees empty screen without error.
#         Maybe add a fallback error message?"

# GitHub建议语法:
# ```suggestion
# let result = try? parse(response, fallback: .defaultValue)
# ```

团队中的代码审查工作流程

有效的审查工作流程基于四个阶段。第一——作者准备PR：写一个清晰的名称（例如"feat: add password reset screen"），添加更改描述、指向跟踪器中任务的链接以及测试说明。第二——作者通过auto-assign（基于CODEOWNERS）或手动指定审查者。

第三阶段——审查者检查代码并留下评论。第四——作者进行修改，回复评论并请求重新审查。循环重复直到获得批准。批准后，作者执行合并（或由机器人执行合并）。通过Mergify或GitHub Auto-merge的自动化加速了最终阶段。

工作流程的重要元素——过时PR管理。如果PR超过3天没有审查，流程会被阻塞。解决方案：审查者轮换（如果指定的审查者不可用），通过Slack/Teams通知，审查时间限制（SLA）。在一些团队中，超过7天没有审查的PR会自动关闭，作者在与main同步后创建新的PR。

• 创建PR——清晰的名称、描述、指向任务的链接、UI更改的截图。
• 指定——通过CODEOWNERS自动分配或手动选择1-2名审查者。
• 审查——按顺序检查：架构→逻辑→测试→安全性→风格。
• 修改——作者回复所有评论，修复阻塞问题，请求重新审查。
• 合并——批准和CI通过后，作者或机器人执行合并。

代码审查的常见错误

第一个错误——肤浅的审查。审查者粗略浏览diff，不深入逻辑，然后点击Approve。原因：大PR、截止日期、疲劳。后果：bug进入生产。解决方案：如果没有时间进行高质量审查——诚实地写"今天无法检查，请推迟到明天"，而不是正式批准。

第二个错误——过度批评（nitpicking）。审查者留下数十条关于格式风格、变量命名、琐碎细节的评论。这会打击作者的积极性并延长审查。解决方案：StyleGuide和linter应自动检查风格。人在审查中检查逻辑、架构和安全性。

第三个错误——没有问题的审查。如果审查者只给出Request Changes和Approve，但不提问，就失去了学习新东西的机会。代码审查健康的最佳指标是存在双方都能学到新东西的讨论。如果审查是其中一方的独白——那么流程已经出了问题。

• 肤浅审查——没有深入理解的Approve。解决方案：没时间就不要审查。
• Nitpicking——批评由linter检查的风格。解决方案：自动化style checks。
• 个人看法——"我会以不同的方式写"。解决方案：代码要能工作，而不是让审查者喜欢。
• 拖延——审查超过24小时。解决方案：审查的SLA，违规时升级。
• 忽略上下文——不理解任务就审查代码。解决方案：在查看diff之前阅读PR描述。