超级管理员
楼主
代码评审容易变成两种极端：要么走过场点个赞，要么纠结于代码风格吵半天。聊聊怎么让它真正有用。
先看什么 按重要性排序：
- 正确性。这段代码做了它声称要做的事吗？边界条件处理了吗？错误路径覆盖了吗？
- 设计。这个改动放在这里合适吗？有没有引入不该有的耦合？抽象层次对不对？
- 可维护性。三个月后别人（包括你自己）能看懂吗？命名是否准确？
- 安全性。有没有注入风险？权限检查到位吗？敏感数据有没有泄露到日志？
- 性能。有没有明显的 N+1 查询？循环里有没有重复计算？
风格问题（缩进、括号位置）应该交给格式化工具，不占评审时间。
几个具体的检查点
错误处理
- 空值/空集合处理了吗？
- 异常是被正确处理还是被吞了？
- 错误信息对排查问题有帮助吗？（「操作失败」这种毫无价值）
并发
- 共享状态有没有保护？
- 锁的粒度合理吗？有没有死锁可能？
- 异步代码里 await 漏了吗？
数据
- 输入验证了吗？
- 数据库操作有没有事务保护？
- 大批量操作会不会把内存打爆？
测试
- 有测试吗？
- 测试验证的是行为还是实现？（测实现的测试一重构就全红）
- 边界条件测了吗？
怎么给反馈
- 区分「必须改」和「建议」。明确标注，别让作者猜。
- 说清楚为什么。不只说「这里不好」，要说「这里在并发场景下会有竞态，因为……」
- 对事不对人。评审的是代码不是写代码的人。
- 不确定就问。与其猜测，不如直接问「这里为什么这么写？」
几条建议
- 小批量。一次评审超过 400 行，发现问题的能力断崖式下降。
- 作者先自审。提交前自己过一遍 diff，能过滤掉大部分低级问题。
- 描述清楚。PR 描述里写清楚「改了什么、为什么改、怎么验证的」。
- 别拖。评审请求放着超过一天，上下文就凉了。
- 自动化能做的别让人做。格式化、lint、依赖检查、测试覆盖率，都交给 CI。
最后一句 评审的目的不是找茬，是共同对代码质量负责。把评审当成对抗，团队就输了。
本文由奇幻维度内容助手整理发布，欢迎在评论区交流你的看法。