taste-code-review

taste-code-review

“能运行”只是底线。按步骤审查，而不是凭感觉——没有方法的 Agent，审查时也不会记住经验。

1. 审查顺序（按此顺序修复，绝不颠倒）

1. 行为——它是否完成了预期功能，包括边界情况（空状态、错误路径、重复点击、离线）？只适用于正常流程的功能，等于披着功能外衣的 bug。
2. 结构——代码是否放在合适的位置？（文件／组件边界、每个单元只负责一项职责、没有 400 行的函数、导入整洁、没有死代码。）
3. 风格——命名、重复代码，以及与代码库惯例的一致性。读起来像没人要求写的诗，比读起来像公交时刻表还糟——不过，站名描述清晰的公交时刻表才是最佳选择。
4. 安全——注入（HTML／URL／SQL）、代码或日志中的机密、每次变更操作的授权检查、不可信输入边界。
5. 性能——明显的算法浪费、N+1 查询、每次渲染都重复的工作、关键路径中的超大负载。
6. 无障碍——键盘操作路径、焦点状态、对比度、语义、屏幕阅读器名称（风格与无障碍检查清单同样适用于代码）。

之所以按此顺序审查，是因为结构性修改会使逐行审查失效——先审查整体，再看细节。

2. 问题发现（按严重程度排序，写得便于采取行动）

• 每项发现都应包含：严重程度（阻断 / 应修复 / 细枝末节）、位置（文件:行号）、一句话说明问题、一句话说明修复方法。没有位置或修复方法的审查只是抱怨，不算审查。
• 阻断：行为错误、安全漏洞、数据丢失、破坏设计系统（参见 taste-code-craft）。
• 应修复：重复代码、死代码、命名偏离、缺少边界处理、性能隐患。
• 细枝末节：格式、命名偏好——可以指出，但绝不能作为阻断项。
• 对于问题更少的差异，发现数量绝不能超过约 10 项。 如果所有问题都是阻断项，那是跳过了审查顺序，不是审查本身。

3. 差异审查纪律（审查始于编写代码之前）

• 审查最小的逻辑变更：把重命名、功能和 bug 修复混在一起的 PR，就像穿着风衣的三个 PR——应拒绝这种打包方式，保留代码。
• 把差异看作一系列决策，而不是一连串改动：“为什么要这样做”比“它做了什么”更重要。
• 生成的代码也要遵守相同标准——生成代码的 Agent 不能因此免于审查。
• 也要审查测试：只验证正常流程的测试是宣传文案，不是测试。

4. 最后一个问题

在批准之前，先问自己一个问题：如果这是我自己写的，我会合并吗？ 如果答案是否定的，用一句话说明原因。如果答案是肯定的，审查就可以通过，并附上问题发现——不必事事完美，但每件事都必须经过深思熟虑。

5. 检查清单（出现以下情况时，审查不通过……）

• 问题发现未按严重程度排序，或缺少位置。
• 未记录边界情况检查（空状态／错误／重复／离线）。
• 因为“这只是一个落地页”而跳过安全检查。
• 未检查代码是否符合代码库现有惯例。
• 对变更行为没有测试，或测试只覆盖正常流程。
• 审查生成代码差异时采用了低于人工编写代码的标准。
• 审查只说“看起来不错”，却没有完成上述各项检查。