资讯动态

开放式代码审查实践:让Code Review不再流于形式

发布时间:2026/10/19 14:01:47 来源:尧图企业网站定制
先问一个问题你们团队的 code review是不是已经退化成合并按钮上的仪式了PR 一提上来reviewer 打开页面看到 CI 是绿的随手点个 approve合并完事。我以前也是这么干的直到有一次线上事故查出来问题恰恰出在我审查通过的那几百行代码里——那一刻我意识到不是我们的审查流程不够快而是这个流程压根没有在审查。所以我花了两周时间在团队里做了一套开放式代码审查实践起了个名字叫 open-code-review。核心思路很简单把审查从一个人挑另一个人的毛病变成整个团队共同对一段代码负责把规则、清单、意见分级和回应机制全部公开透明地写下来。这篇文章就把这套东西完整讲清楚包括我是怎么设计的、落地时踩了哪些坑、效果到底如何。适合正在被 code review 形式化问题困扰的工程师、技术 leader以及任何想把代码质量命令式提升一把的人。1 传统 code review 为什么会失效三个真实场景复盘1.1 橡皮图章式审查批阅的不是代码是流程我们团队之前是标准的 GitHub flow所有 PR 必须至少一个人 approve 才能合并。听起来有个流程但实际发生的画面是这样的早上十点小张把 PR 发到群里配一句求 review然后大家都忙着写自己的代码没人管。到了下午三点小张又催了一遍这时候我作为团队的 tech lead 不好意思了打开 PR 页面扫了一眼 diff看到改动不多、CI 也过了就点了个 approve。这个流程的问题在于我把审阅变成了清空通知。我没有看这段代码在什么场景下调用、异常处理是否合理、边界条件有没有覆盖、测试有没有真正断言逻辑而不是断言实现。我在意的是让这个 PR 尽快合入而不是这段代码是否足够好。后来线上出事故的那个 PR正好就是这么审过去的。改动是一个配置中心接入逻辑看起来很简单——读配置、存缓存、返回结果。问题出在缓存没有设置过期时间而且失败路径上没有降级。这两件事只看 diff 是看不出来的你得知道这个接口的调用方对实时性有多敏感得知道如果配置中心挂了会发生什么。这些信息不在 PR描述里reviewer 也不会主动去问。就这样一个肉眼可见的隐患在审查通过后上线直到配置变更后直接炸了。1.2 战斗式评审意见成了立场代码变成了战场和橡皮图章相反的另一端是另一种极端。团队里有个资深工程师人称行走的 linter他审查代码的方式是从头到尾质疑每一个变量命名、每一个 for 循环的写法、每一处缩进。他的评论数量常常超过 PR 自身的代码行数而且语气体量不是建议是你这样写不对。结果就是代码作者每次提 PR 之前要做很长时间的心理建设提完之后就陷入防守状态。两个人你一句我一句到最后已经不是技术水平之争而是面子之争。有几次甚至要拉我出来断案。这类审查文化带来的问题比不审查还严重——它让开发者养成了回避反馈的习惯。作者学会了用重构完再提来延迟暴露learned 用更长、更大的 PR 来减少评审次数甚至学会了在 merge 之后才在群里说早知道应该....这种马后炮。1.3 两者共同的病根没有统一的审查目标和规则我把两种现象放在一起对比发现病根是同一个团队从来没有定义过好的审查应该长什么样。reviewer 的职责边界是什么意见怎么分级哪些必须改、哪些可以讨论作者怎么回应意见没有人说清楚。所以 open-code-review 做的第一件事就是把这些问题全部答案写成文档让所有人都看到。不是每个人都认可但至少有了共事的基础。这就像写代码之前先定 coding convention不是为了限制谁而是为了让争论有边界。2 open-code-review 的核心机制目标、原则和运行方式2.1 一句话定义 open-code-review 是做什么的open-code-review 不是某个特定工具也不绑定某个平台。它是一套面向开发团队的开放代码审查运行机制包含三部分内容一份公开的审查清单每个 reviewer 按清单逐项检查而不是凭感觉发挥。一套四档意见分级规则把必须改和可讨论分开避免风格之争淹没逻辑问题。一套作者回应机制让每条意见都有明确归属要么采纳、要么解释、要么计划下期处理不允许意见挂在那里烂尾。需要强调开放两个字的意思。这里的开放不是指源码对外开放而是指审查过程和规则对团队全员透明。任何成员都可以 review 任何人的代码都可以对既有规则提出修改建议。规则本身是开放的有人提意见说清单某一条不合理大家可以讨论修改不是某一个人说了算。这套机制跑在常规的 PR 流程之上GitHub 或者 GitLab 都能用不需要安装什么神秘插件也不强迫你从一个工具迁移到另一个工具。它改变的是人的行为方式而不是工具链。2.2 设计原则审查者不是警察是过滤器我设计 open-code-review 时脑子里反复出现一个类比审查者不应该像一个红绿灯警察看到违章就开罚单而应该像机场安检的过滤器目标是发现高风险物品而不是对每一个乘客的穿搭做评价。这两个角色的区别体现在权重分配上。警察模式是看到什么挑什么只要和规范不符就提出来过滤器模式是先判断这段代码如果出问题代价有多大再决定投放多少关注度。同样是几十行代码放一个通信协议解析器和放一个简单的列表渲染审查者投放精力的密度显然不应该一样。因此清单里面每一类检查项都标了权重。正确性和设计问题占大头可维护性问题其次风格问题全部交给自动化工具去管人工不看。这样一来reviewer 的精力集中在真正影响上线质量的问题上不会被芝麻细节分散掉。2.3 运行闭环从提 PR 到合入的五个强制步骤有了原则还得有强制动作。我在 open-code-review 里规定了每个 PR 从创建到合入必须走完的五个步骤作者自检提 PR 之前先按清单自己过一遍把自检结果写在 PR 描述里。指定至少一个 reviewer且 review 必须在 24 小时内完成首轮反馈。reviewer 按清单逐项检查产出分级意见。作者逐条回应意见回应结果同步在 PR 页面上。全部 P0 意见合入前必须关闭P1 意见允许合并但必须在三天内补上修复。这套闭环最关键的其实是第 1 步。作者一旦知道要自检并且自检结果要展示在 PR 描述里他就得多一次思考很多低质量问题在提 PR 之前就被自己干掉了。3 落地实操审查清单、意见分级和回应规则的具体设计3.1 审查清单五类二十条每条都可以照做清单是 open-code-review 最核心的抓手。我最初整理的时候从网上、内部分享和个人经验里收集了大概 80 条然后按照收益/成本过滤最后留下二十条。太少了覆盖不全太多了没人会逐条去看。二十条刚刚好。清单分成五个维度每个维度四条维度检查项正确性1. 空指针/越界等基本异常路径是否处理2. 并发场景下共享状态是否有保护3. 外部传入的数据是否做了校验4. 错误处理分支会不会导致状态不一致设计合理性5. 职责划分是否清晰有没有一个函数做太多事6. 是否引入了不必要的新依赖7. 可扩展性如何三个月后需求变化时是否好改8. 边界场景的处理是否和上下游约定一致可维护性9. 命名是否准确表达了意图10. 有没有重复代码可以复用已有实现11. 注释是否解释了为什么而不是复述做了什么12. 日志是否足够支持问题排查测试质量13. 测试是否覆盖了核心逻辑的正常路径14. 是否有边界条件测试15. 测试断言的是行为还是实现细节16. 测试运行时间是否在可接受范围性能与安全17. 是否有明显的性能隐患如 N1 查询、循环内 IO18. 敏感数据是否被正确保护和脱敏19. 是否存在内存泄漏或资源未释放的场景20. 变更是否会影响线上系统的稳定性实际用的时候还有两条补充做法。第一不要求每条都必须通过清单是检查框架不是及格线每条给一个通过/不适用/需要讨论的三态标记reviewer 可以标不适用跳过。第二清单本身放在仓库里作为文件任何人都可以提 PR 修改它——这就是开放二字在规则层面的体现。3.2 意见分级P0/P1/P2/P3 四档每一档对应不同行动比清单更重要的是意见分级它解决了评审里什么都重要什么都不重要的困境。级别含义行动要求示例P0合入后会引发线上事故或安全漏洞必须合入前修复未鉴权的接口、可导致数据丢失的操作P1逻辑明显错误或与需求矛盾但当前环境不触发允许合并三天内修复数组越界但当前数据规模不会触发、错误日志记录级别错误P2建议改进不影响合入作者自行决定是否采纳命名可读性、重复代码提取、补充注释P3讨论性质不要求行动reviewer 和作者对话交流换个思路设计会不会更好某段逻辑和另一个模块是否有关联这个分级表写出来后团队里最大的变化是吵架变少了。以前都会争你这个评论是 blocking 还是 non-blocking现在看到 P2 就知道不用上纲上线到不能 merge的程度看到 P0 也知道不能为了合入而妥协。意见的分级写清楚之后reviewer 对自己的发言负责作者也更容易接受。实操中我还加了一条铁律分级是发表意见的默认义务不允许只写一个建议修改就完事。每条意见必须给出级别没有级别视为无效意见作者可以不回应。这一条把很多仅供参考其实没什么信息量的评论直接过滤掉了。3.3 作者回应机制回应是义务反对要有替代方案很多人写完代码最讨厌别人指指点点但 open-code-review 里作者没有沉默权。每条意见必须有一个归属状态采纳、不采纳或有后续 action。具体操作是reviewer 发意见后作者 tag 一下回复可以写同意已修改可以写这里我不同意原因是...请再看一下如果意见有理但现阶段改不了那就写记下了下个迭代处理已经建 issue。这个流程不像听起来那么重因为大多数意见确实一句就能回应完但这几句话让它产生了闭环。意见有了归属就不会出现我提了十几条你一个都不理的挫败感也不会出现作者嘴上说改实际忘了的烂尾。我还给作者设了一个保护条款反对某条意见时不能说我觉得没问题必须给出替代方案才能拒绝。这个替代方案本身可以是暂时不改观察线上效果——关键是要有一个明确的判断依据不能都凭感觉。3.4 规则公开后的意外收获新成员上手速度明显变快这套清单和规则公开之后团队来了两个新同事培训 code review 相关的成本几乎归零。以前跟新人解释我们团队怎么 review 代码要讲半小时还要看他悟性。现在直接把 open-code-review 的文档链接甩过去新人在提第一个 PR 之前就能按清单自检reviewer 给的意见也按分级一目了然不需要额外的文化灌输。这也是我坚持把规则文档化的一个隐藏收益——隐性知识一旦变成显性文本团队对它的依赖就变得可复制、可沉淀、可迭代。即使有人离职这套规则还在。4 程序配置和工具链接入不需要另起炉灶改造现有流程即可4.1 分支保护规则、CODEOWNERS 和 PR 模板配置之前说得比较偏文化层面但 open-code-review 也有一部分是可以在工具配置里落地的。我用的是 GitHub就以它为例。首先是分支保护。在仓库的 Settings Branches 里把 main 分支的保护规则打开勾选Require a pull request before merging把Require approvals设为 1。这一条至少从流程上保证没有人能绕过 review 直接推代码。不过这里有一个常见误区——需要 review 不等于 review 有效所以还要配合下面两条。然后是 CODEOWNERS 文件。在仓库根目录建一个 .github/CODEOWNERS让核心目录的代码必须由对应 owner 来批准。例如# .github/CODEOWNERS /src/core/ tech-lead backend-core /src/api/ api-owner /docs/ tech-writer这样设计的目的不是搞层级审批而是确保关键路径上的代码一定有能看懂它的人在旁边看着。核心模块改错了可能整个系统都崩这个层面的去 ping 那一下的价值大于大部分日常 review。然后是 PR 模板。在 .github/PULL_REQUEST_TEMPLATE.md 里放一个结构化的说明让作者提 PR 时直接填空## 变更内容 !-- 这段 PR 做了什么为什么做 -- ## 自检记录 - [ ] 异常路径已考虑 - [ ] 测试覆盖核心逻辑 - [ ] 不涉及敏感数据 - [ ] 命名清晰无重复代码 - [ ] 日志可支撑问题排查 ## 影响范围 !-- 依赖此模块的其他服务有哪些 -- ## 特殊说明 !-- 如果 reviewer 需要重点看什么地方写这里 --这一步表面上看起来是流程增加实际上是在给 reviewer 提供上下文。没有这些信息reviewer 看一段陌生的 diff 和一个陌生的模块效率极低。4.2 CI 先过滤机械问题人工审查只干人应该干的事open-code-review 里面有一个很关键的边界划分凡是机器能判断的事情不管 review 怎么好都别人工去做。格式、缩进、变量命名规范、单测覆盖率阈值、基础静态检查这些全部交给 CI 来卡。我们团队在 CI 里挂了 golangci-lint、eslint、prettier 检查以及一个自定义的单测覆盖率脚本。任意一项不通过PR 右下角直接显示红叉连 review 环节都走不到。这样做的好处是人工审查意见里永远不会有你这个地方空格不对这种噪音。意见数量少了每一条的含金量反而上去了。有人可能会担心CI 管太多会不会拖慢合入速度。实际上正好相反从长远看CI 是不睡觉的、不情绪化的、可以并行跑的它卡掉的问题不需要人来等而人工审查的精力被释放出来后就可以集中在比风格深一层的问题上。4.3 同步审查和异步审查的搭配15 分钟的集中评审时段代码审查也分同步异步。异步就是传统的非实时 reviewreviewer 有空了打开 PR 评论一句同步是大家约一个时间聚在屏幕前逐行过一遍。我一开始只做异步发现一个问题异步交互的回合数一多PR 挂几天都动不了——你中午提个问题作者下午才看到回复完又得再等半天。后来改成每个工作日固定 15 分钟把两到三个待审 PR 投屏出来全员快速过一遍重大问题当场讨论小问题记下来异步处理。这个集中评审时段配合异步 review效果出奇地好。集中时段解决的是评审节奏问题异步通道解决的是深度问题。不过 15 分钟时间里只做点名式过场不适合深入看设计。所以我规定集中时段只看两类问题一类是 P0/P1 风险的确认另一类是作者主动提出需要讨论的点。其余问题一律回到异步通道。这样15 分钟够用大家也愿意来开这个会。4.4 一次典型的 PR 生命周期演示拿一个实际的例子来串一下整个流程。假设后端同学提交了一个新增加密字段的解密接口的 PR作者写自检清单里 20 条过了一遍勾选异常路径已考虑、测试覆盖核心逻辑在 PR 描述里写依赖 auth 模块的 token 校验需要重点看权限部分。指定公司同事做 reviewerGitHub 自动通知。reviewer 打开 PR按清单逐个维度检查。发现接口没有对传入的用户 ID 做归属校验任何人可以传别人的 ID 拿到对应的解密数据——给出 P0 意见。作者看到 P0先回复确认已加归属校验顺手补了一个测试。CI 全绿P0 关闭approve合并。三天后作者按承诺补了日志优化把 P1 关闭。整个流程跑下来最长的一条 PR 从提起到合入不超过两天。对比以前一个能挂一周的节奏提升是实打实的。5 推进过程中踩过的坑这些问题不改规则再好也没用5.1 文化准备不足有人把提意见当成被否定open-code-review 刚铺开的时候阻力最大的不是流程本身是情绪。有个同事平时话不多但写代码一向仔细。第一次被人按清单逐条 review他收到四条 P1 意见整个人状态明显不对在 PR 评论区跟 reviewer 一来一回呛了十几条。后来我找他单聊发现他的真实想法是这些代码我调了两天你上来就说有三个问题是不是觉得我不行这个心结代码层面解决不了只能从机制层面化。所以我做了三件事一是在周会上公开说review 到的问题是代码的问题不是人的问题二是让包括我在内的每一个核心成员都带头示范自己收到的每条意见都用规则回应谁也不搞特权三是每次新人入职的第一周我亲自给他提第一个 PR教他怎么看意见、怎么回应而不是丢一份文档就完事。这个过程比较慢但跑了一个月之后大家真的逐渐把意见当成免费的设计评审而不是攻击。5.2 时间预算没给够没有评审时的硬保护另一个踩得很痛的坑是光规定了 24 小时内必须反馈但没给 reviewer 留出时间。大家都忙着自己的迭代你的审查要求在我这儿排不上优先级于是又回到明天再说的状态。解决办法是把这个时间纳入迭代排期。我们在每个迭代计划里给工程师预留了每天 1 小时专门用于 code review和写代码的时间等同对待。这个看起来只是一个排期变化但它传达了一个信号审查是被认可的正式工作不是加班义务。这一点对于让流程长期运转特别重要靠自觉不是长久之计。5.3 存量代码没人碰新规则全砸在新 PR 上这不合理还有一件事是我之前没想到的规则一上线所有审查火力全集中到了新建的 PR 上但系统里几百个早已存在的模块没有任何人去看等于整套流程只保护增量不碰存量。这个问题的后果是新旧代码之间的质量差距越拉越大很多技术债被推迟处理却不被记录。我的调整方案是在 open-code-review 的体系里增加一个存量代码复审的节奏每个月留出半天由团队选一个业务模块全员一起过一遍核心路径。这样做的收益不只是发现存量问题更是让所有新人熟悉老模块最好的途径spoken 的 knowledge sharing 也就藏在里面了。5.4 只审不修意见大量产生行动项却悬挂最后这个坑发生在规则运行到第六周的时候。我看了一下周报发现 P1 意见被采纳的数量不少但真正在三天内修完的不到六成。原因是修 P1 没有硬性卡的环节人一忙起来顺手就把自己的承诺忘了。针对这个问题我把 P1 的修复时间纳入了 GitHub issue 的提醒机制任何 P1 意见reviewer 在打标签的同时自动建一个 issue 并指派给作者截止日期三天后到点自动提醒。这一改P1 的关闭率在一个迭代内回到了九成以上。5.5 关于审查人数的误解人多不等于 code review 质量高最后想说一个常见的误区。很多人认为 approver 越多代码质量越有保障所以把分支保护的 Required approvals 从 1 调到 2、调到 3。我们的经验是超过 2 个之后边际收益急速下降甚至可能变成负的——因为每个人都默认别人会看结果每个人都没细致看这就是社会心理学里的责任分散效应。open-code-review 里我更推崇的是一个主 reviewer 全员有知情权的模式。主 reviewer 对合入负责必须认真地逐条过一遍清单其余人如果有精力可以补充讨论但不需要所有人都 approve。这就避免了责任摊薄也保留了团队透明性。6 用数据说话跑完 12 周后质量指标到底怎么变6.1 三个硬指标的变化规则跑了 12 周我做了一次定量的复盘。取了我们团队最核心的三个指标对比 open-code-review 上线前 12 周和上线后 12 周指标上线前上线后变化线上缺陷逃逸率每千行0.420.19下降 54.8%上线后回滚次数41下降 75%PR 从创建到合入的平均时长62 小时28 小时缩短 54.8%看到 PR 平均时长缩短的时候我是有点意外的——按直觉加了这么多检查应该变慢才对实际上是之前流程里大量存在PR 挂一周没人理、想合合不了的隐性等待时间被 24 小时首轮反馈机制干掉了。6.2 有一个指标变差但我认为这是好事有一个指标是变差的让我一开始很紧张评审意见总数从平均每条 PR 3.2 条涨到了 7.8 条。表面看是问题变多了但拆开分析后发现P2 和 P3 这类讨论型意见占比从 20% 上涨到了 65%而 P0 的数量并没有明显变化。这说明什么说明团队开始把评审当成设计讨论的场所而不仅仅是纠错工具。意见更多了但大多数不是你错了而是我们还有别的选择。用这个视角来看7.8 条意见不代表这代码烂而代表这个团队对这段代码的理解在加深。6.3 团队感受适度和过度之间的平衡最后说一下团队的主观反馈。匿名调查里有 80% 的成员认为 open-code-review 比之前的流程更有价值12% 觉得差不多8% 觉得更麻烦。那 8% 的意见集中在一个点上觉得分级表格记起来麻烦每次都要翻文档。这个反馈促使我把表格做成了浏览器插件式的速查卡放在我们团队内部 wiki 顶部又做了一页打印版贴在工位上。虽然听起来很小但确实把这个摩擦降下来了。所以说流程设计不要想着一步到位要留出持续优化的空间。规则本身必须是开放的才能在真实使用中被检验、被修改。我在开发和使用 open-code-review 的过程中最大的体会是工作流再造这件事最难的永远不是写出规则而是持续迭代规则。任何一个流程刚出来都不会完美只有允许参与者对规则本身提出质疑并共同演进它才能在一个团队里真正活下来。如果你的团队也正在被 ineffective code review 困扰不妨先挑清单和分级这两件最关键的事开始剩下的可以边跑边调。

读完文章,也想定制专属网站?

尧图设计师 24 小时内与您沟通定制方案

免费获取报价 →
↑