
1. 为什么大多数代码审查最终沦为点赞机先说一个我在团队里观察到很多次的场景PR 一提交过了一会儿被人点了 Approve评论只有一个单词 LGTMLooks Good To Me然后代码合入主干。表面上看流程走了、记录有了、门禁也过了但实际上这一整个环节里没有发生任何有效的审查——没人认真看代码没人在意设计取舍更没人发现里面埋着的问题。这不是某一个人的问题。我接手过好几支团队也见过不少开源仓库情况惊人地一致代码审查从一种质量保障手段慢慢退化成了一种形式主义仪式。仪式当然要有但仪式本身并不能保证质量。真正的问题在于我们把审查当成了一项每个工程师天生就该会做的事但事实是——大多数人根本不知道审查的时候到底要看什么。1.1 三种典型的假审查行为我归纳过团队里最常见的三种假审查你可以对照一下第一种是扫一眼型审查。审查人打开 diff快速滚动一遍没看到明显的语法错误或者大的结构问题就点了通过。这种审查最大的盲区是它只能发现眼前的问题发现不了缺少的东西。比如边界条件没处理、错误路径没覆盖、日志信息不完整这些在 diff 里不会自动跳出来。第二种是安全区型审查。审查人心里其实看到了问题但觉得这个改动是别人花了很多心思做的我提意见会不会显得我在挑刺这是我的同事写的改了会不会影响关系。于是话到嘴边又咽了回去最后只挑了一个无伤大雅的命名问题说出来好像是给了意见其实什么也没说。第三种是事后道歉型审查。代码在一周前已经合入了现在项目出故障了回滚之后大家开始在群里翻旧账当时我就不该放它过。但问题是当时的审查记录里没有任何人提出异议。这三种情况背后的共同点是审查环节没有给参与者提供足够的支撑结构。大家不是不想做好是不知道做好到底意味着什么。1.2 流于形式的四个结构性原因如果只是个别团队的问题那可能是团队文化的问题。但当一个现象在整个行业普遍存在我就倾向于认为这是结构性的。我总结下来原来团队审查失效主要卡在四个点上第一缺少审查标准。审查人打开一个 PR 的时候心里没有任何检查依据。他只能凭个人经验和临场感觉去判断。而个人经验这个东西波动太大了。有人擅长看并发问题有人擅长看数据模型有人只关注命名和代码风格。没有一个统一的该看什么的基线审查质量就完全取决于当天审查人的状态和水平。第二职责边界模糊。谁是本次改动的第一责任人审查人到底要对什么负责维护者又在什么情况下有仲裁权这些在大多数团队里都没有定义清楚。结果就是提交者觉得反正有人审查我可以随意点审查人觉得我只是给建议采不采纳是别人的事最后一旦出了问题没有人对代码负责。第三缺少反馈闭环。很多审查意见提出来了但是散落在评论里没有人跟进到底哪些改了、哪些没改、为什么没改。等 PR 合并之后这些意见就彻底丢失了。下一次遇到同样的问题还得从头再犯一遍。第四没有时间预算。很多团队把审查当成把手头的事放一放、抽空去看看的副业。结果就是审查周期被无限拉长。而人的心理有个很现实的现象一个 PR 等得越久大家越倾向于快速放行因为已经等了太久不想再耗了。1.3 反直觉的判断问题出在设计而不是人在我最开始想解决这个问题的时候我的第一反应是做思想工作跟团队成员说大家要重视审查、要认真看代码。结果没什么用。后来我才想明白一个反直觉的道理代码审查流于形式问题不在人的态度而在流程设计本身。人的决策系统天然倾向于省力。如果你的流程没有一个明确的方法论告诉他第一步看什么、第二步看什么、什么情况需要停下来讨论、什么情况可以直接放行他一定会选择最省力的方式——快速扫一眼然后通过。这就像你在厨房里做饭如果菜谱只写了加盐适量新人一定是一边加一边尝最后不是淡了就是咸了。但如果你直接告诉他两斤食材加一茶匙盐出错概率就会小得多。代码审查也是一样我们需要把适量变成可执行的步骤。所以后来我做了 open-code-review 这个开源项目目标很直接把代码审查从凭感觉变成按流程走让任何一个工程师打开一个 PR 的时候都知道自己到底该干什么。2. open-code-review 的顶层设计把审查变成一件有标准可循的事open-code-review 不是那种重量级的代码审查平台。市面上的 Serene、Reviewable 这类工具我也用过功能很强大但对中小团队来说往往过度设计光是配置和上手成本就吓跑了一半人。我做这个项目的初衷是提供一个**规范 脚本 模板的轻量框架**它不替代现有的 Git 工作流也不强制你换平台而是嵌入到 GitHub、GitLab、Gitea 这类常见的代码托管平台里直接在 Pull Request / Merge Request 的流程中运转。2.1 项目组成与使用场景如果你现在打开 open-code-review 的仓库会看到三个核心部分审查清单模板按变更类型分类的 CheckList这是整个框架的基石。发布之后你可以直接复制到自己的仓库里用也可以按团队情况修改。辅助脚本一组轻量脚本用于在提交时自动计算 diff 规模、识别高风险文件、生成审查摘要。这些脚本都不依赖外部服务用 Python 或者 Node 就能跑。流程约定文档角色定义、时效约定、仲裁机制、意见跟踪方式。这部分是软规范配合清单使用让团队内部有共同的游戏规则。整个项目设计上遵循一个原则清单能解决的绝不用脚本脚本能解决的绝不用平台。一个团队把这三样东西用起来通常只需要一个下午的时间。2.2 三层审查清单的设计逻辑清单是整个机制里最核心的设计。我把它分成三层每一层解决不同的问题。第一层通用工程质量项。不论你改的是前端、后端、脚本还是配置文件有一些问题是共通的。比如变更是否包含对应的测试错误处理路径是否完备是否有调试代码遗漏console.log、print、debugger依赖变更是否在文档里同步说明命名是否准确表达了意图这类问题就放在第一层每次审查都必须过一遍。第二层类型专项项。不同领域的关注点完全不同。改 Java 后端的时候你关心事务边界、并发安全、SQL 性能改 React 前端的时候你关心组件卸载时的内存泄漏、依赖数组、状态一致性改数据库迁移脚本的时候你关心回滚方案、数据兼容性。每个方向都有专属的检查项。这一层需要团队里面资深的技术人针对自己的领域去沉淀而不是从网上下载一份通用清单就能覆盖的。第三层团队自有约定项。这是最容易被忽略但又最有价值的一层。每个团队在过去半年都出过线上事故、踩过坑那些坑往往就藏着最好的检查项。比如某个团队被未做幂等导致重复支付坑过一次那本次接口是否考虑了重复请求的幂等性就应该是这个团队的专项检查项。这些东西是任何外部工具都没法给你的只能从自己的切肤之痛里沉淀出来。2.3 为什么用清单而不是自由评论很多人一听到审查清单就觉得太死板、太教条觉得写代码是件创造性工作不该被条条框框约束住。我在早期也有过这种想法。但后来我在认知心理学里看到一个概念解释了清单为什么有效。人的注意力是有限的。当你在审查一个三百行的 diff 时如果脑子里同时要在意性能、安全性、可读性、架构一致性、边界条件、依赖管理你的工作记忆很快就会过载然后自动放弃深度检查转入快速浏览模式。看到哪算哪想到什么说什么。而清单的作用是把一定要看什么从大脑里卸载到纸面上。审查人不再需要一边看代码一边在脑海里搜索我该注意什么他只需要拿着清单一项一项核对。这个过程看起来很机械但恰恰是这种机械性保证了审查的覆盖度不会因为审查人当天的状态而波动。另一个很实际的好处是清单给了新人一个最低起点。一个刚入组两个月的开发他可能没有足够的能力发现深层次架构问题但他完全可以拿着清单逐项核对比如这个 PR 里有没有包含测试有没有调试代码。等他积累了足够的经验他自然会有能力跳出清单看到更多到时候他会自己增加检查项。清单不是天花板而是地板。2.4 项目的模块划分在 open-code-review 的仓库里我按下面的结构来组织open-code-review/ ├── checklists/ │ ├── general.md # 通用工程质量清单 │ ├── backend-java.md # Java 后端专项 │ ├── frontend-react.md # React 前端专项 │ ├── database.md # 数据库变更专项 │ └── template.md # 团队自定义清单模板 ├── scripts/ │ ├── diff-stats.py # 计算 diff 规模与文件分布 │ ├── risk-file-scanner.py# 识别高风险文件配置、核心模块、依赖 │ └── review-summary.py # 生成审查摘要便于逐项记录 ├── docs/ │ ├── roles.md # 角色定义与职责边界 │ ├── workflow.md # 完整审查流程说明 │ └── arbitration.md # 意见分歧的仲裁机制 └── pr-template/ └── pull_request_template.md # 集成清单的 PR 模板这个结构对应的使用场景很清晰你拿到项目后不用全盘照搬可以先从checklists/里的通用清单开始跑用template.md建立自己的专项清单然后配置好pr-template/让开发者在提交 PR 的时候就看到清单要求。至于脚本属于锦上添花前期跑不起来也可以先不依赖。3. 团队落地实操从第一天就能跑起来的审查流程工具设计得再好落不了地也是白搭。我见过太多团队把代码审查平台配得很漂亮最后照样流于形式。所以这一章我直接讲落地步骤你照着做从第一天就能在真实仓库里跑起来。3.1 初始化把空仓库变成带审查机制的项目假设你现在的仓库是空的或者是一个还没有任何审查规范的老仓库第一步不要急着推流程先做三件事。第一件建docs/code-review/目录把审查清单放进去。我建议把 open-code-review 的通用清单复制进来再让团队里的技术负责人按自己领域的专项清单做取舍。这个动作的意义在于晾晒——清单放出来所有开发者都知道审查时会用哪些标准要求他们不用等到被问了才恍然大悟。第二件配置一个最简单的 PR 模板。以 GitHub 为例在仓库的.github/目录下创建pull_request_template.md内容里带上清单的链接并且明确要求提交者在 PR 描述里逐项自检。比如这样## 变更说明 用三句话描述这个 PR 要解决什么问题 ## 自检清单提交前逐项确认 - [ ] 是否包含相应测试如果本次变更无法测试请说明原因 - [ ] 是否包含调试代码/临时日志/硬编码配置 - [ ] 是否更新了相关文档README、API 文档、环境变量说明 - [ ] 是否考虑兼容性数据库迁移、接口参数变化、前端缓存 - [ ] 是否按团队规范完成格式化与命名 ## 关联记录 - 需求/缺陷单号 - 设计文档链接你可能觉得这个模板有点老生常谈但我实测下来清单嵌入 PR 模板的效果远远好过单独放在文档里。因为提交者在写 PR 描述的时候就必须看到这些内容他没法说自己不知道有这个要求。第三件配置分支保护规则。在 GitHub 或 GitLab 里把主干分支设置为必须通过审查才能合入。这个步骤很多人会做但如果你的团队规模很小建议先别配置必须通过 CI这类强门禁只配置必须有至少一个 review approval否则小团队会等到天荒地老。3.2 好的审查需求小贴士这一节其实是给那些想跳过基础配置、直接深入定制的人写的。你在跑通用清单一两周之后往往会发现有些检查项在你的团队里根本不适用有些很重要的项又没出现在清单里。这时候就需要定制了。我的建议是从过去三个月的事故和回滚里挖素材。把团队在最近一个季度里出现的线上问题、紧急回滚、加急修复都拉出来一个一个过。每发现一个如果当时有人问一句就不会出事的点就把它变成一个检查项。比如线上出了事务边界问题加上检查项本次变更中涉及事务的代码异常回滚是否完整数据库连接是否在 finally 中释放外界反馈某个接口性能低到超时加上检查项本次接口调用是否评估了最坏情况下的响应时间是否存在 N1 查询的必要性有人发布时把环境配置改了影响线上服务那就加本次变更是否修改环境配置如果改动了配置有没有在发布说明里备注这些从事故中提炼出来的检查项比网上任何公开清单都更有针对性。它会把团队过去的痛苦转变成未来的阻挡网。再提醒一句每个清单的条数要克制。我的经验是通用清单加专项清单总条数控制在 15 条以内。超过这个数审查人会疲劳最后变成机械打勾反而有害。3.3 角色绑定与时效约定流程只有清单还不够必须有角色定义。open-code-review 里把参与审查的人分成三重角色提交者Author是代码的第一责任人。在发起审查之前必须先完成自检清单的逐项确认并且在 PR 描述里写明设计取舍和已知风险。一句话提交者要把自己的代码当作品来交付而不是先发出来再说。审查者Reviewer对变更正确性负责。他要做的事情是站在测试者和用户的角度逐项确认并明确给出结论Approved / Needs Changes。不写感觉这个可以这种模糊评论。维护者Maintainer在意见分歧时做最终仲裁并且对合并时序负责。不是每个 PR 都需要维护者出现只有审查者与提交者无法达成一致的时候才需要介入。除了角色时效规则一定不能省。没有截止时间的审查就等于没有审查。open-code-review 里我建议的默认值是角色响应时限说明审查者首次回应4 个工作小时内就算不能马上完整审查也要先回应已收到将在 X 时间前给出意见完整审查完成1 个工作日24 小时内这是上限不是目标越快越好提交者回复意见4 个工作小时内逐条确认是否采纳同意或不同意都要有理由变更后复审4 个工作小时内小改动在复审后即可合并避免二次排队这些时间设定不是拍脑袋定的而是基于一件工作如果超过 24 小时没有反馈之前投入的上下文就基本作废了这个事实。你在自己的团队里可以调整但务必要有一个明确的时间底线否则流程很快就会回到无限期挂起的状态。3.4 一个完整 PR 的审查过程演示光讲理论不如看一次实际跑流程。我模拟一个典型的例子方便你理解整个环节是怎么串起来的。假设团队里的前端同学小王要提交一个修复搜索框输入过慢的 PR。第一步提交前自检。小王在 PR 描述里勾选了自检清单在是否包含相应测试这一项他如实写了新增了防抖函数的单元测试未做端到端测试因为开发环境暂未配置。第二步自动脚本检查。代码推到远程之后CI 里跑起了 diff-stats 脚本输出这个 PR 的 diff 规模是 287 行涉及 3 个文件没有改动核心模块风险等级为常规。第三步审查人介入。团队里的前端组长小张收到提醒后在 24 小时内开始了审查。他打开 diff对照前端专项清单逐项确认没有发现组件卸载后 setState 的警告防抖的时间参数来自常量而非魔法数字对输入框清空时的处理有覆盖性能监控的日志是否会被频繁触发——这里他提出了疑问。于是小张在评论区写了一条这里的性能日志记录在 input 事件里吗如果用户连续输入 5 秒日志会刷多少次建议加一个节流或者只在超过阈值的场景下上报。第四步提交者回应。小王看到评论回复确实是我最初只在超过 500ms 时记录但后来重构时不小心把它挪到了主逻辑里我改一下。然后他补了一次 commit。第五步复审与合并。小张看了新 commit确认问题已经修复点了 Approve。小王随即合入主干。整个过程大概花了 20 个小时其中真正消耗人力的是二十分钟的审查和五分钟的修复。相比以前那种挂三天没人理的 PR这个节奏已经是天壤之别。4. 跑通之后的一个月数据变化与踩坑记录流程不能跑起来就算完你要能拿出来数据证明它有效。我在两个团队里试点过这套机制每个团队大概 6 到 8 人跑了一个月之后统计了一些关键指标。4.1 试点团队的真实指标变化我让人把试点前后的数据拉了一张对比表指标跑之前一个季度均值跑之后一个月均值变化提交到首次审查的响应时间约 38 小时5.5 小时明显缩短合并前平均等待时间约 3.5 天约 1.2 天缩短近三分之二至少发现一个有效问题的 PR 占比约 12%约 53%提升明显合入后 30 天内因缺陷回滚的概率约 9%约 3%明显下降审查评论中非阻塞意见占比约 40%约 17%主观感觉讨论更聚焦这些数据当然不是严格意义的对照实验但方向上很说明问题给审查人一个明确的方法和清单他们会真的开始发现问题。最让我意外的是合并前平均等待时间竟然下降了。我本来以为加入审查环节会更慢但结果恰恰相反——因为流程明确了响应时间有承诺反而没有人再用在等别人回复当拖延的借口了。4.2 个坑清单膨胀导致的审查形式化跑流程的第二周我就发现了一个问题审查人开始把清单当成必答题而非检查工具。我在 checklists 里放了很多很有价值的检查项但每一条都要求审查人勾选加起来有二十多条。结果就是审查人翻完代码之后直接快速勾选所有是根本没有真正逐条核对。这个问题我自己的体会是清单的设计是对实际情况的妥协你放进去的每一项都要承担审查人注意力的成本。如果一项检查内容在 90% 的 PR 里都完全不适用它就不应该出现在通用清单里而应该挪进专项清单或者删除。后来我做的调整是把清单按必须生效项和按变更类型匹配项分开。所有的变更必需看的安全类、测试类检查项留在通用区域那些仅当改动涉及数据库时有效仅当改动涉及接口时有效的检查项必须由提交者在发布PR时标注自己的变更类型审查人只核对相关区域的检查项。这样单次 PR 里真正需要看的检查项被限制在 8 条以内效率一下就上来了。4.3 第二个坑强制门禁在小型项目里的束缚另一个很快冒出来的问题是把一个只有 100 行的脚本改动或者 README 更新也放到同一条严格的审查流水线里确实很蠢。我试过一个团队里有人改了一个 Markdown 文档也要走提交 PR、等审查 24 小时、复审、合并的路子结果发布一个文档要花两天时间人人怨声载道。解决方式我称之为分级审查策略。把变更分成三档第一档文档、配置注释、非功能性改动——单人审查审查人可以是改动者自己相邻领域的同事半个工作日完成即可第二档常规功能改动、Bug 修复——一个审查者 明确的清单匹配第三档高风险模块改动支付、用户权限、数据迁移、主链路——两个审查者 必须执行全部专项清单 维护者确认。这一套下来既保证了关键路径的安全性也没有拖累普通改动。你不需要做个非常智能的系统来识别档位最简单的办法就是在 PR 模板里让提交者自己勾选本次变更属于哪一档并在描述里说明理由。如果抽查发现有人故意把高风险改动标注成低档再单独沟通纠正大部分人是老实的。4.4 第三个坑新成员对审查意见的防御心理流程跑顺之后我发现有个更深层的问题浮了出来有几个新同事看到审查人给自己提了十几条意见第一反应不是我需要改这些而是是不是组里觉得我能力不行。有一个同事甚至在群里公开问是不是我写的代码太差了才需要这样看这个问题不解决再好的清单和流程都会变成人与人之间的心理负担。我后面在如何提审查意见的文档里加了三条硬性要求事实优先不用主观评价。不要说这个代码写得不好而是说这段逻辑在 X 情况下会走不到这里因为 Y 判断挡住了它。指出影响再给建议。不要只说这应该改要说明如果不改用户连续点击时可能出现重复请求导致支付单创建两次。让建议跟着问题走而不是显得在挑刺。区分阻塞项和非阻塞项。如果某个改动不是必须修的明确标注非阻塞供参考让提交者知道哪些意见是不定死线的降低心理压力。把这些规则写进流程文档之后新成员对审查的态度明显放松了。他们开始把审查意见当成别人替自己兜底的一个保障而不是否定自己劳动成果的判决书。4.5 我在多个团队反复强调的一个附带收益这里我想额外分享一个我在跑流程之后的发现那就是审查记录变成了一份宝贵的团队知识资产。以前我们做经验沉淀靠的是写 wiki、写文档但很少有人去看。可审查评论不一样它被存放在 PR 页面里和具体的代码、具体的改动绑定在一起。当团队里有人遇到相似场景的时候他只需要在历史合并记录里搜一下幂等或者超时这类词就能看到当时针对这类问题大家达成的共识。这份知识比任何培训都来得直接。所以在跑流程的时候我强烈建议团队约定在 PR 合并之前把关键的审查结论归档到固定的目录或文档里比如docs/adr/。一次审查也许只花几分钟但沉淀下来的结论可以反复使用这是很多团队从未意识到的收益。5. 开放协作场景下这套机制如何放大价值这套机制在内部团队里跑顺之后我很快意识到它的价值远不止于此。真正让它发挥更大效能的地方是开源社区和跨团队协作场景。5.1 开源维护者如何用同一套机制管理社区 PR有开源项目经验的朋友都知道维护者在社区 PR 上最头疼的事情有两件一是外部贡献者的代码质量参差不齐又没办法像内部员工一样手把手教二是维护者自己时间有限不可能每份 PR 都从头到尾逐行审。open-code-review 的设计恰好能缓解这两个问题。你在仓库里放好清单和 CONTRIBUTING.md 之后外部贡献者提交 PR 时就会看到你的要求。它可以清晰地告诉贡献者你要过这五关而不是等维护者看完一遍代码之后才给出缺少测试缺少文档这类亏欠。前者是流程引导后者是事后审判体验完全不同。具体的落地方式我已经在真实项目里验证过在 CONTRIBUTING.md 里明文写出对 PR 的检查要求并在 GitHub Action 里用脚本做一次初筛。这个初筛脚本可以不检查代码逻辑只检查模板是否填写自检清单是否照常勾选diff 规模是否超过预设阈值。这样维护者收到的每份 PR都是至少走完了基本流程的过滤掉大量明显不规范的贡献。5.2 跨团队协作时的边界划定当代码仓库被多个团队共同时直接应用同一套审查标准往往会起冲突。比如 A 团队要求所有变更必须有两个以上审查者B 团队觉得这是给他们的临时配置改动增加负担A 团队有一套安全红线B 团队的人并不知道。我在带跨团队项目时采用的方法是把审查项分成红线区和协作区。红线区由核心维护团队定义任何团队的代码必须遵守例如不得将密钥明文提交、生产环境配置需按规范变更、数据库迁移必须有回滚脚本。这些项用脚本强制检查和归档不依赖于某个审查人是否忘记勾选。协作区则给每个模块的可能性留足空间由各模块自己的主负责人和审查团队约定其他团队的人介入协作区块时只需要遵循基础清单不需要研究对方团队的所有历史沉淀。这个做法既保证了核心安全又不强迫所有参与者学习对方的所有规则。5.3 下一步可以做的事很多但这件最值得跑这套流程喂出来很多可以继续挖掘的数据。比如你能通过审查意见的分布看出哪些模块最容易出错、哪些审查者的意见接受率最低、哪些问题的反复出现率最高而值得变成自动化的静态检查。目前 open-code-review 的辅助脚本还只是做了 diff 统计和风险文件识别这些基础工作未来最值得投入的方向是把审查记录里的意见做成可检索的知识库让一个团队在代码接手时不只是拿到一份代码同时能拿到这个产品在这块领域踩过的所有决策。这其实也是我最终想做 open-code-review 的初衷代码审查本身不该只是找错它应该成为团队用来学习和绕坑的基础设施。一套流程好不好不该只看它挡住了多少坏代码更该看它有没有帮助一批人形成更好的写代码习惯。我在自己的项目里跑了它大半年最真切的体感是代码里的坑永远不会绝迹但有了一套能让大家都用的审查规则之后掉进同一个坑里的人确实少了很多。如果你所在的团队还在为代码审查流于形式发愁我建议你不妨先把这套框架里的清单拿回去试试改一改、跑两周你会看到数据的真实变化。