code-review

Code Review

概述

对代码做有重点、分层次的审查——先抓正确性和安全问题，再管可读性，风格交给工具。核心：好的 review 让作者知道"哪里有真问题、该先改什么"；坏的 review 是一堆 nit 淹没重点，或一句"看起来不错"放过错误。

何时使用

• 提交前审查自己或别人的代码（自我 review 见下文专门技巧）
• 同事让你看他的 PR
• 不确定 review 该看什么、怎么反馈

不该用：纯风格/格式问题（交给 linter/formatter 自动化，不该占用人 review）；想确认"功能对不对"——那是 verify-and-fix（实际跑）的事，review 是静态审查、替代不了运行验证。

与相邻 skill 的边界：code-review 是审查视角（找问题、分严重度、给建议），verify-and-fix 是修复视角（改对、验证）。code-review 发现的问题，需要修的进入 verify-and-fix 流程；review 中发现的"行为不对/报错"可能要先用 debugging 定位根因。三者接力：review 找问题 → debugging 定位 → verify-and-fix 修+验证。

核心内容

正确性优先，别被风格带偏

review 最常见的错是逐行挑风格（命名、缩进、注释多少）而放过正确性（逻辑对不对、边界处理了吗、有没有竞态）。正确性问题会让程序出错，风格问题只是不好看——前者 critical，后者 nit，优先级天差地别。

按重要性排序，review 该查的维度：

1. 正确性：逻辑对吗？能跑通预期路径吗？有没有逻辑漏洞？（最该花时间）
2. 边界与异常：空值/空集合/零/负数/超大输入怎么处理？外部依赖失败（网络/DB）呢？
3. 安全：有没有注入（SQL/命令/XSS）？密码/密钥处理对吗？权限检查到位吗？敏感信息泄露吗？
4. 并发：共享状态有竞态吗？异步顺序依赖对吗？资源泄漏（未关闭的连接/锁未释放）？
5. 可维护性：命名清不清楚？结构是否过度复杂？有没有重复？（这里才是 nit 的领地）
6. 测试：有测试吗？测的是行为还是实现？覆盖了关键路径和边界吗？

前 4 类是"会让程序出错或出事"的，必须查；第 5 类是"让人难受"的，次要；风格细节（缩进/格式）不该人查，交给工具。

按改动性质调整重点：上面是通用清单，但不同代码该重点查的不同——别对所有代码平均用力。涉及钱/库存/计数的，重点查原子性和一致性（中途失败会不会凭空产生/消失）；涉及外部输入的（用户输入、API、文件），重点查安全（注入、越权）；涉及共享状态/异步的，重点查竞态和资源泄漏；涉及配置/迁移的，重点查回滚和兼容。先识别"这段代码的风险面在哪"，把 review 力量集中投到那里。

分严重度，别把 nit 和 critical 混着

review 反馈必须分严重度，让作者知道先改什么：

• 阻塞（blocking / critical）：必须改才能合并——逻辑错、安全漏洞、会崩溃、数据丢失风险。
• 重要（important）：强烈建议改——边界没处理、缺少测试、设计有隐患，但不阻塞本次合并。
• 建议（nit / suggestion）：可选——命名、可读性、小重构。改了更好，不改也能过。

不分层的 review 有两种失败：把 nit 当 critical（作者被一堆小事压垮，反而漏改真问题）、把 critical 当 nit（真问题被淹没在风格意见里）。nit 要克制——堆 15 条 nit 是在浪费作者时间，把同类 nit 归并成一条"风格建议"，或直接交给 linter。

反馈要带依据和建议，不只是"这不好"

每条 review 意见应该让作者能理解问题 + 知道怎么改：

• 指出问题 + 为什么是问题：不说"这写得不好"，说"user.name 在 user 为 null 时会抛 TypeError——db.find 找不到时返回 null"。
• 给方向或示例：不只说"改一下"，给"加个 null 检查，找不到时抛业务错误或返回 null，看调用方期望"。
• 区分事实和偏好："这里有 null 风险"（事实，基于代码）vs "我觉得该用 early return"（偏好，标注是建议）。

带依据的反馈让作者能判断对错、学到东西；空泛的"不好"让作者只能盲从或抵触。