★ FEATURED ARTICLE 开头“open-code-review”这个名字起得其实挺妙两个层面都讲到了代码审查这件事本身是开放的——任何人的改动都可以被团队里的同伴审阅、讨论、修订再合入主线与此同时工具链也应该开放——不依赖某一家商业平台的封闭流程而是用 Git 本身加上一套可自定义的脚本、配置和约定搭出属于自己团队的质量防线。我见过很多团队把代码审查做成“过场”PR 一开链接丢群里第二天 10 条 approved谁也没仔细看。也见过反过来的极端——审查严到像法庭质证行行都是“为什么这里不用 switch”改代码的人被批到怀疑人生。这两条路我都走过最后沉淀下来的这套玩法可以概括成一句话用开放的心态搭流程用封闭的规则卡质量线。这篇文章就把我从零搭建代码审查流程的过程完整拆开。核心内容包括为什么必须做审查、审查流程怎么设计、工具和脚本怎么配、一次完整审查该怎么操作以及最常见的五个坑和对应的排查方法。不管你是刚带小团队的技术负责人还是想把个人项目也做一遍自审的独立开发者这套思路都能直接用。1. 代码审查到底解决什么问题1.1 不只是“找 bug”而是让代码有人负责很多人对审查的第一反应是“多点几双眼睛找 bug”这个说法没错但只说对了一半。代码审查更大的价值是逼着每一行代码都有一个“明确的主人”。你想想看自己写的代码过了三个月再回头看你大概率认不出其中某段逻辑当初为什么要写成这个样子。如果是别人写的那更麻烦——功能报错了翻到那个函数注释是“// fix edge case”边缘情况是什么边缘情况为什么是这么修的统统没说。这时候你会从心底里骂一句这代码谁写的可 Git blame 一查赫然是自己的名字。审查就是在这个“写成不久、记忆还热”的时间窗口里把上下文强制说清楚。我在这边要求每个 PR 的描述里必须写明白三件事改了什么、为什么改、怎么测过。这三句话不写全直接打回。理由很简单如果提交者自己都说不清这三个问题说明他自己都没有真正理解自己的改动。代码有问题可以改但思路混沌的改动改十遍也一样混沌。反过来审查人也不是闲着。旧代码有人审新代码就有人看看懂了才点批准看不懂就发问。等这条 PR 合入之后代码的上下文就不再只存在写代码的人脑里而是变成了团队所有人都能看懂的公共知识。1.2 为什么我把“审查”做成一套流程而非单次活动早年间我也干过“集中审查”的事——攒一个大版本开个会拉上三四个人投影仪一开从头捋到尾。结果怎样前 20 分钟大家还在认真看后 20 分钟只剩一个人在讲另外几个人开始刷手机。三个小时后几个人精疲力尽结论是“整体还行有几个小问题我回头发你”。这种操作的本质问题在于审查压力在时间和空间上过于集中人根本扛不住。代码审查本质上是一个持续的过程而不是一个时间节点。每一次 commit、每一次 diff、每一次讨论都是审查的一部分。把审查拆成高频、小颗粒、自动化的动作才符合人脑注意力的规律。所以我现在带的任何项目代码审查都遵循三条铁律小的改动优先一次 PR 最好控制在 200~400 行以内超过 600 行就应该拆开。一次吃 1 万个 diff 的审查脑力完全不够用人只会看个大概。机器能做的先做风格、格式、明显逻辑错误让工具去卡节省人工审查的带宽。人工审查只做机器做不了的事关注架构合理性、命名、是否过度设计、是否漏了边界条件和异常场景。这套思路其实和“开放代码审查”的核心精神是一回事让最合适的手段处理最合适的问题人工只用在对的地方。1.3 “开放”代表的另外一层含义透明与可追溯“open”这个词还隐含一个容易被忽视的特质——透明。代码仓库所有人可见每次改动都有据可查谁在什么时候改了什么、为什么这么改在代码历史里一清二楚。透明带来一个很现实的收益团队里不会出现“代码只在某个人脑子里”的情况。核心模块哪怕只有一个作者写过审查记录和交流记录都在仓库里任何人接手都能顺藤摸瓜理解设计初衷。透明还天然抑制了那种“悄悄绕过流程”的做法。改一行密文提交、顺手 r 一下这种事在有完整审查记录的系统里很难发生因为每一次例外都明晃晃写在历史上。2. 从零搭一套代码审查流程2.1 先定分支模型审查的地基流程设计里第一件事不是配工具而是把分支模型定下来。分支模型决定了改动以什么形态进入主线也决定了审查触发的时机。我用过好几种模型最痛苦的是“全员主干开发”——所有人直接往 master 上推审查无从谈起。稍微规范一点的是经典的分支 合并请求模型功能分支从主干拉出开发完成之后通过合并请求或拉取请求合回主干审查就挂在合并请求上。至于更复杂的、多级分支互相合并的模型——比如开发分支上面套发布分支——适合大型多版本并行项目。个人项目或者十人以下的小团队我建议直接走最简单的单一主干加功能分支主线保护开启任何人不能直接推只有通过审查的合并请求才能合入。关键是分支模型要适配团队规模不是为了复杂而复杂。我踩过的最大的坑是把两三个人的项目硬套大公司的双层分支模型结果所有人的时间都花在合并分支上真正的代码审查反而没精力做。2.2 最小闭环里的五个环节我搭的这套流程可以把一次审查拆成五个环环相扣的节点缺一个都不完整环节触发条件负责人核心动作自检commit 推送前提交者本人跑本地测试、静态检查、自查 diff自动化门禁PR 更新时CI 与脚本语法、格式、测试、覆盖率门槛人工审查PR 创建后指定的审查人逐 diff 阅读、评论、讨论修改反馈收到审查意见后提交者本人逐条回应、修改、重新推送合入与归档审查通过后审查人或维护者squash 合入、删除分支、关联 issue这套闭环的每一步都对应一个可以量化的指标——比如自检要跑完哪些命令、门禁必须通过哪些检查、审查人必须在多少小时内给出首轮反馈。这些指标不一定写进正式文档但应该是团队里每个人都默认遵守的约定。我见过有团队把 CI 门禁搭得极其豪华一个 200 行改动流水线上跑了镜像打包、多环境部署、全量回归一套全跑完要 40 分钟。结果是开发者的半小时都被耗在等流水线上审查密度反而大大下降。所以我的原则是门禁只卡机器能快速判断的事慢速事情放主干合入之后再跑。2.3 PR 与在线评审的组合拳先回答一个问题为什么我在工具上优先选择在线评审而不是面对面评审。原因很简单在线评审留下的是异步、可检索、可追溯的讨论记录。面对面会议上关于某行代码的争论开完会就散了后面的人完全不知道当时为什么这么定。反而是弹幕式、评论式、逐行标注的在线审查记录半年之后回看依然有参考价值。但这不意味着线下评审就毫无意义。新成员入职、架构级的大改动、跨模块的牵扯讨论这几类场景我还是会拉一个小范围的会。做法是先在线发起 PR大家先花 30 分钟把 diff 读完然后会议高效讨论分歧点会后再把结论整理成在线评论回填到 PR 里。这样既有异步的沉淀又有同步的碰撞效率。3. 把规则固化成工具和清单3.1 Git Hook 做第一道红线人工审查之前得先把“低级错误”挡在门外。这里我强烈建议在仓库里启用客户端 Git Hook。合入之前先拦一道审查人的时间就不会浪费在格式化、缺分号、提交信息乱七八糟这种问题上。我平时在 pre-commit 钩子里挂三样东西代码格式化工具按项目语言选比如风格统一器字段排序或者导入排序检查提交信息格式校验。钩子脚本写在仓库.githooks目录下然后让每个开发者执行git config core.hooksPath .githooks来启用。这一步配置完所有提交都会统一走这套检查。一个常见的坑很多新人 clone 完项目之后没有执行 hooksPath 配置于是钩子形同虚设。后面我发现与其靠每个人的自觉不如把这条命令写进项目 README 的“快速开始”章节并且把本地启动脚本做成安装依赖 配置 hooks 初始化环境一条命令跑完。这样新环境的容错率至少高了一个量级。3.2 在 CI 上加自动化门禁客户端钩子能防君子却防不了漏配。所以更硬的一道保障在服务端 CIPR 分支一推送流水线自动跑起来检查不过直接标红没有人为绕过的余地。我给小型项目配的最简单门禁是这么一套编译或语法检查确保改动不破坏构建单测执行全量跑一遍失败即阻断覆盖率阈值全项目覆盖率不得低于基线或者本次改动不得显著降低覆盖率格式检查与本地钩子同一套规则防止本地漏跑。这里要说一个经验覆盖率阈值这个指标数值本身不重要趋势才重要。我见过团队把阈值定到 90%然后为了凑数字写了一堆断言空方法的废测试。覆盖率的意义是锚定“没测过的地方要知道”不是拿数字去考核。所以阈值我一般定在“不稳定回退”而不是“越高越好”。另外不完全依赖 CI 的一个细节流水线默认只跑和本次改动相关的模块还是全量回归我的建议是 PR 阶段跑相关模块 全量编译全量回归放到主干合入后的夜间流水线。这样既保证了审查时的心智负担小又不会放过主干集成问题。3.3 把审查清单做成团队的“共识文档”人工审查最怕的就是“凭感觉”。每个人的风格偏好不一样A 觉得解构赋值就是优雅B 觉得显式变量更好读这种争吵是最消耗团队能量的。解决的办法是把争论提前到文档层面而不是在每条 PR 里临场发挥。我定义了一份可以落到仓库里的审查清单PR 描述里自动带上审查人按清单逐项过功能逻辑与需求是否一致有没有偏离需求之外的“顺手改动”边界条件和异常分支是否考虑到了命名是否表意清晰能不能做到“读代码即读注释”是否有重复逻辑可以抽象复用是否存在性能隐患不必要的循环、未关闭的连接、无界查询安全性初检用户输入有没有走校验、敏感信息有没有落日志测试是否覆盖了核心路径和至少一个边界情形这份清单的价值不在于每项都必须完美通过而在于它把“什么是好的代码”变成了一种公共语言。审查时不用笼统说“这代码感觉不太好”而是挂到清单某一条上“第 4 条这几段循环可以合并成一次遍历健壮性且可读性都会好一些”讨论立刻具体、可执行。3.4 模板化 PR 描述给 PR 准备模板这件事看起来是形式主义实际上功效不小。一个没有模板的 PR描述常常是空白的或者只有一句“fixed bug”。这种 PR 的审查人只能自己去 diff 里推断改了什么时间成本非常高。我用的模板要素很精简改动目标这个 PR 想解决什么问题改动方式核心技术方案涉及哪些文件/模块验证方式本地怎么测的测试用例有哪些知识点备注可选设计上有哪些值得讨论的点模板本身放在仓库里PR 创建时自动带入。审查人看到描述里的改动目标和改动方式基本就能快速定位要看哪里如果描述写得含糊直接打回重填。这个规则坚持一个迭代周期之后PR 质量肉眼可见上升。4. 一次完整审查的实操记录4.1 提交前的自检不只祈祷“能跑”真的下笔写这个部分之前先承认一件事我早期提交 PR 之前也是焦头烂额跑一遍测试就推上去然后等 CI 红了再慢慢补。现在我自己有一套固定的提交自检步骤用顺了之后效率反而更高。本地自检四连编译与单测全绿跑一遍静态检查把警告从上到下过一遍能修的修不能修的想清楚为什么不修并写在 PR 描述里自查 diff切到暂存区状态把改动文件逐一打开对着差异片段再想一遍“这段代码今天看得懂三周之后还看得懂吗”重新读一遍 PR 描述确认没有遗漏改动目标。这步看起来花时间但其实能省掉大量和审查人来回拉扯的时间。一次把话说清楚比反复“补充说明”体面得多。4.2 审查人视角diff 不是全部拿到一个 PR很多审查人的第一反应是直接展开 diff 一行行看。但最高效的顺序是反过来的第一步先读 PR 描述搞清楚改动目标和方式 第二步看改动涉及的核心文件先理解主要逻辑变化再回头补次要文件 第三步逐行看 diff重点标注逻辑边界和状态变化 第四步结合测试改动检查测试是否覆盖了主要行为。为什么是这个顺序因为 diff 本质上是一个“局部的、混乱的”信息流——它把所有文件拆成行级别打乱理解成本极高。先在大脑里建起“改动地图”再看 diff 才看得懂。我在审的过程中会刻意区分必要评论和建议性评论。必要评论指的是不改代码就无法合入的问题——逻辑错误、严重性能退化、安全问题建议性评论指的是“如果是我我会怎么写”这一类偏向个人偏好的意见。两者要分开写否则提交者面对 20 条评论神态会从感激直接变成麻木。另外一个实用技巧评论里用问题而不是判决。与其写“这里不应该用数组”不如写“这里用数组是否有额外的内存开销换成迭代器是不是更贴合场景”这种问句式的反馈目的不是展示审查人的水平而是引起提交者的思考。大部分合理的修改建议提交者在思考后自己就会提出来比我直接教他怎么写效果好得多。4.3 评论、修改与合入的节奏审查中最消耗信任感的一种情况是提交者把 PR 一丢人消失了审查意见在下面躺了三天无人问津。所以我在团队里约定了一条硬规则审查人发完首轮评论后提交者必须在当天最晚次日给出逐条回应。回应的方式有两种认同说明打算怎么改然后推送新 commit不认同解释自己的理由双方讨论达不成一致时上升到第三个仲裁人而不是在评论区互刷 20 条。这个“第三仲裁人”的设定一开始很多人不习惯觉得这不是制造矛盾嘛。但实践中它恰恰是效率最高的兜底机制。有争议不可怕可怕的是争议没有终结机制最后变成个人恩怨。合入环节小团队的项目我都建议开“squash merge”——把功能分支的杂乱提交压成一条干净的提交再合并到主干。主干历史保持线性、语义化每个 commit 对应一个完整的可工作状态。这会让后面看 git log 或者回滚时体验好很多。4.4 用自动化机器人补上人工盲区纯人工审查还有一个盲区是人对“重复模式”不敏感。比如全项目里同一段相似逻辑出现 5 次审查人看第一次会认真想看到第五次已经麻木。这种时候工具反而比人可靠。我在 CI 里加了一个轻量级的“重复代码检查”步骤阈值调得比较宽松只标记明显重复加上一个小规模的复杂度检测超过复杂度的函数挂警告。这两个指标不是考核项而是提示项——出现告警不拦截但在 PR 评论里标出来让审查人心里有数。顺带提一句这类分析工具如果误报率高会很快失去大家的信任。我的经验是先本地样本上跑一遍把噪音阈值调好再上 CI。直接裸上只会收获一片“工具又在瞎报”的骂声。5. 常见问题与排查技巧实录5.1 审查效率太低一个 PR 拖一周怎么办这个问题几乎每个团队都会遇到。核心原因是没有为审查留出明确的排期和阻塞规则。我尝试过几个方案最后比较好用的是“双 DDL 制”提交者创建 PR 时给一个“期望合入日期”审查人被指派后必须在 24 小时内给出首轮意见哪怕是“看得差不多了还差两处我想确认”超过 24 小时没有动静提交者有权提醒超过 48 小时可以升级到项目维护者处理。听起来很刚性但实际执行下来大家都松了一口气。因为审查本身不是没有意愿而是永远在跟“优先级更高的事情”抢时间。有了明确 DDL审查就成为了一个被认真对待的日程项而不是无限期悬置的待办。另外效率优化有个结构性手段轮值审查人制度。每两周轮流指定两个固定审查人团队其他成员的 PR 优先分派给他们。这样审查技能会有积累——固定审查人对项目上下文更熟平均审查时间会显著下降。猛烈批评此段的问题在于有些小的 PR 也可能找不到人来审轮值制度需要兜底。5.2 评论里情绪升温矛盾要怎么化解代码审查最内耗的场景不是工作量大而是讨论变质。我遇到过一次审查人觉得提交者“为了炫技把代码写得过度抽象”提交者觉得对方“看不懂高级写法就乱打枪”评论区你来我往最后上升到人身攻击。后来的处理方式是私聊拉通一次双方各自明确表达了自己的代码价值观才把气氛缓解下来。从那之后我在团队里立了三条审查礼仪对事不对人评论对象永远是“这段代码”而不是“你”先认同再建议如果有可取之处先说出来分歧升级机制超过三轮讨论没有收敛不再继续在评论区纠缠直接私下沟通或拉第三个人进来。代码风格的冲突本质上是品味的冲突很多问题没有对错。与其花一小时说服对方“我这样更好”不如花五分钟确认团队统一的编码规范。这也是我在 3.3 节强调清单文档的另一个原因——有统一规范很多争论根本不会发生。5.3 自动化门禁误报太多导致开发者直接绕流程门禁工具滥用的情况我见过不止一次。最典型的是格式检查某次升级之后旧文件全被标成风格不合规PR 打开一片红开发者怒从心头起干脆git commit --no-verify硬推。所以自动化门禁有一个很重要的设计原则可执行的快速修复不可执行的就升级为提示不要红牌。比如格式问题我应该提供的是“一键修复”而不是“告诉你错了然后让你手工改”。覆盖率也是同理不是红线硬卡而是标黄提示。评论里给一条说明“这次改动的核心路径覆盖还行但异常分支没测”比 CI 红牌更能引导开发者改进。再有门禁规则本身的变更要有孤立机制。升级工具、新增检查规则时先在独立分支跑一遍全仓库统计误报率确认可控之后再布到主流水线。这样就能避免“升级一时爽全员骂娘”。5.4 新成员不知道该审什么刚加入团队的新人往往是最认真的审查者同时也是最迷茫的审查者——打开 diff 不知道从哪里下手评论不敢发只能默默点个 approve。针对这个问题我给的解法是让新人先做“观察式审查”前两周不要求新人独立审 PR而是让 TA 跟着一位主审查人一起看目的不是找问题而是理解团队对于代码的取舍偏好同时让新人从测试审查入手。只看测试文件不看实现代码测试写得够不够全面。这个切入点比较低门槛很快能建立自信再下次让新人审文档类和配置类 PR这些变化范围小、逻辑清晰容易做深。等新人完成了三轮这种梯度式练习再放给 TA 一个正常的功能 PR质量就基本在线了。6. 一些收尾的经验体会6.1 审查的本质是一种沟通方式做了这么多年代码审查最大的一个体悟是好的审查不在于抓到多少个问题而在于让每个参与者都愿意在下一次继续认真对待它。代码审查最理想的状态是团队里形成一种默契提交者认真准备、审查者认真阅读、分歧被公开讨论并形成决策。在这个氛围里代码质量只是副产品真正沉淀下来的是团队对于“什么是对的”的共同认知。所以我坚持把审查记录、决策理由、讨论过程都留在仓库里——它们就是团队文化的史官。6.2 最后分享两个小技巧先说一个我屡试不爽的技巧审查自己的 PR 之前先隔一晚。前一天写的神级抽象逻辑第二天早上看可能就是一团乱麻。给自己留一点“冷却时间”再去读自己的 diff很容易发现自己当时的盲点。条件允许的话可以先合并主干再审查这样可以消除主干分叉带来的干扰。另外一个技巧审查别人的代码时先点开这个文件的 Git 历史。看看这个文件之前经历了什么改动往往能帮你理解当前代码为什么长成这个样子。很多时候你以为的不合理是有历史包袱的。审人的时候带一点这种同理心比直接开喷要容易建立信任得多。这套流程搭起来不难真正难的是让每个参与者都从中获得正向反馈。作为技术负责人也好作为团队里的资深成员也好如果你能让大家在审查结束之后觉得“这次讨论有收获我的看法被认真对待了”那么这套流程自然会运转得越来越顺畅。代码审查不是方向盘的拦截装置它是团队这辆车的后视镜加导航仪——方向对了它帮你稳稳开得更远。
阅读完成 · 觉得有帮助？
