评审标准
从相关的角度去评审。不是每个角度都适用于每次改动。自己判断。
代码真的做到了意图说它该做的事吗？
- 边界情况：空输入、nil 或 undefined、边界值、并发访问
- 错误处理：错误是被捕获了、往上传了，还是被悄悄吞掉了？
- 差一错误、类型隐式转换、整数溢出、字符串编码
- 状态管理：竞态条件、过期的闭包、悬空引用
- 正常路径能走通吗？出错的路径能走通吗？
- 幂等：这个操作跑两遍会怎样？上一次跑到一半崩了又会怎样？如果答案是「要看留下了什么状态」，就说明少了一步把状态对齐的处理。
- 并发：如果多个参与方能碰到同一份可变状态（文件、分支、共享数据），访问是靠结构保证串行的（锁、按顺序的阶段、独占的归属），还是靠迟早会被打破的约定？
发现一个可能的 bug 时，把执行路径追一遍。不要只标一句「这里可能是 nil」，要写出让它变成 nil 的那条调用链。
根因还是症状
Section titled “根因还是症状”代码是在修真正的问题，还是在掩盖症状？
回答这个问题往往要看改动文件以外的地方。读周边的代码（调用方、被调用方、类型定义、同级模块），弄懂这次改动所在的架构。用你手头的工具（Read、Grep、Glob）去探索。顺着调用链走。读类型。先弄懂代码为什么存在，再判断改动是不是改在了对的那一层。
- 用防卫语句盖住了更深层的 invariant 被破坏
- 用重试逻辑掩盖了一个已经失效的约定
- 用类型转换压住了一个建模上的错误
- 看到 workaround，就问：为什么需要这个 workaround？像样的修法是什么样？
- 修在模块 A 里的东西，其实应该修在模块 B 的约定上
- 本该用结构解决的地方用了说明：如果修法是一条写着「不要做 X」的注释，或者一条得有人记住的约定，问一句能不能改成类型约束、lint rule 或运行时检查，让错误的做法根本无法发生
结构是否完整
Section titled “结构是否完整”代码和它所在的系统合得来吗？
- 边界纪律：校验放在系统边界上，还是散落在业务逻辑里？数据进入系统的地方校验一次，之后在内部信任它。
- 抽象层次：代码有没有把高层的编排和低层的细节混在一起？
- 耦合：这次改动有没有引入会让以后的改动更难的依赖？
- 数据模型是否合适：数据结构和实际的访问方式对得上吗？结构对了，下游代码一目了然。结构错了，处处跟你作对。
- 硬接上去还是融进去：这次改动是补在现有设计上的，还是读起来就像设计一开始就考虑到了？如果一开始就知道这个新需求，代码会是现在这个样子吗？
- 新旧两条路并存：这次改动是不是加了新 API，同时留着旧的？如果没有外部使用方，就在同一轮里把调用方迁过去，删掉旧路径。不要留下会变成永久的兼容层。
不要因为简单的代码缺少抽象就扣分。过早抽象比重复更糟。
读代码能看出它是对的吗？
- 有测试吗？测的是行为，还是实现细节？
- 有没有能抓住退化的断言或 invariant？
- 如果是修 bug：有没有针对这个 bug 的测试？
- 如果碰到了集成边界：整条路径测过吗？
- 查真实的东西，不要查替代指标。如果代码靠文件修改时间或缓存的状态来判断是否存活，而不是去读真实的值，这就是验证上的缺口。
- 对于委派出去的或异步的工作：代码验证的是实际产出的文件，还是相信自我汇报和摘要？
代码做成的事，配得上它的复杂度吗？
- 可以写得更简单、又不损失正确性和清晰度的代码
- 只为一个调用点服务的抽象
- 为还不存在的情况做的配置或参数化
- 死代码、没用到的 import、残留的参数
- 过度设计：「以防万一」的代码路径，目前没有任何调用方
- 为了过渡期的稳定而留着、如今已不再需要的过时兼容路径。迁移做完了，就把脚手架删掉
- 用户体验撑得起这份复杂度吗？每个功能、控件和选项都得证明自己值得存在。做一半的功能比没有更糟。
越简单越好，除非简单是错的。三行重复胜过一个过早的抽象。
每条安全方面的 finding，都要把输入在代码里经过的路径追一遍，并写出来。
- 用户输入没有经过清洗，就流进了危险的去处（SQL、shell、eval、innerHTML）
- 新端点在认证或授权上的缺口
- 代码、日志或错误信息里出现密钥
- 安全关键路径上的 TOCTOU（检查时和使用时之间的时间差）