1. 聊聊代码审查它从来不只是“找茬”代码审查这件事在软件开发圈子里算是个常青话题。隔一段时间就有人跳出来喊“代码审查没用浪费时间”过一阵子又有人分享“我们团队用代码审查挽救了项目质量”之类的经验贴。我在一线写代码写了十几年大大小小的团队待过不少GitHub、GitLab、Gerrit、Phabricator 都用过对代码审查的感情很复杂它确实烦也确实有用。关键在于——大多数团队根本没有在做“真正的”代码审查只是在走形式。先说一个基础认知代码审查Code Review也就是项目标题里的 open-code-review 所指向的核心本质上是“对代码的第二次思考”。你自己写代码的时候思路是线性的、单向的很容易沿着自己设想的路径一路走到底。而审查者拿到代码时脑子里没有你那些“理所当然”的背景假设他们会从边界条件、异常路径、并发场景、可维护性这些角度切入找出你视野盲区里的问题。这不是谁比谁厉害的事情而是“第二双眼睛”本身就有不可替代的价值。这个主题适合谁看如果你是刚入行的开发者想搞清楚为什么团队要搞代码审查、怎么在审查中少被怼如果你是团队里负责推动工程效率的技术负责人想优化现有审查流程让审查不再流于形式哪怕你只是自己写开源项目想让代码质量上一个档次——这篇内容都值得你花几分钟读完。我会把自己这些年实际踩过的坑和有效的做法全部摊开来说不整虚的。2. 为什么很多团队的代码审查形同虚设在讲怎么把代码审查做好之前我想先花点篇幅聊聊“为什么它经常做不好”。因为如果你不知道病根在哪照搬再多的最佳实践也没用。2.1 审查变成“批斗会”人性层面就出了问题代码审查最大的阻力其实不是技术问题是心理问题。写代码是一件很有“创作感”的事情每个人都对自己写的代码有天然的保护欲。如果审查的方式是“你这里写得不对”“这个逻辑有 bug”“你怎么这么写”被审查的一方很容易产生防御心理轻则在心里翻白眼重则当场怼回去。时间一长大家为了避免冲突审查就变成了走过场随便看一眼点个“同意”就完事。我见过最极端的例子是团队里一个资深工程师因为审查方式太强硬硬生生把两个新人逼得离职了。他们不是扛不住压力而是觉得自尊心被反复践踏。代码审查要想长期运转下去第一条铁律必须是“对事不对人”。你在审查意见里说“这个函数的边界条件没有处理”而不是“你怎么连边界条件都不处理”效果是完全不同的。前者讨论的是代码后者攻击的是人。2.2 审查规模和节奏完全失控第二个常见问题是审查的规模和节奏。很多团队的开发流程是这样的新功能开发周期三周代码分支一直往里堆提交直到功能全部做完才提出审查请求。然后审查者打开一看改动列表里躺着 40 个文件、3000 行代码变动瞬间就失去了一半的阅读意愿。这里有个数据可以参考Google 的工程实践指南里明确建议一次代码审查的合理规模是 200 行以内最好控制在 100 行左右。我个人经验也差不多超过 300 行审查质量就会急剧下降。人类大脑的信息处理带宽有限当你的工作记忆被塞满的时候后面看到的代码其实是在“右眼进左眼出”根本注意不到深层问题。规模失控不仅让审查效果变差还会让整个团队对审查这件事产生畏难情绪陷入“改动太大→不想看→草草通过→质量下降→需要返工”的恶性循环。2.3 工具和人工的职责边界没有划清还有一个很隐蔽的问题就是工具和人工的分工混乱。有的团队把代码审查完全当成“人工跑测试”审查者的时间大量浪费在抓拼写错误、命名不规范、缺少空格这类机器已经能自动发现的问题上。有的团队则走了另一个极端觉得反正有自动化测试和静态检查工具人工审查就随便看看结果线上的并发问题、设计缺陷一个都没拦住。正确的心态是凡是机器能做的事情不要让人去做。静态检查工具、格式化工具、单元测试、持续集成管线这些负责的是“客观标准”的部分而人工审查的核心价值在于“主观判断”——设计是否合理、接口是否优雅、是否存在隐藏的边界问题、未来维护起来顺不顺。这个边界划不清审查的投入产出比永远不可能提上去。3. 一套高效代码审查流程的设计思路既然问题清楚了那么解法也就呼之欲出。我这里不打算推荐某一个特定平台的流程而是给出一个通用的、可裁剪的框架你在 GitHub Pull Request、GitLab Merge Request 或者 Gerrit 里都可以落地。核心目标只有一个让审查省时、高效、不招人烦。3.1 前置防线提交前把能自动化的都自动化在很多团队里代码审查是从开发完成、提交 PR 那一刻才开始的。但我的经验是真正的审查应该从“提交前”就启动。你要做的不是在审查阶段抓低级错误而是把低级错误全部过滤掉让人工审查的注意力集中在真正需要人脑判断的问题上。我推荐的最小前置防线是这样的提交前本地跑一遍格式化工具和静态检查比如 Python 的 black ruff、Go 的 gofmt go vet、前端项目里的 ESLint Prettier确保没有风格类问题。能写单元测试的地方尽量写提交时把测试跑通。CI 管线里配置好构建和自动化测试PR 一提交就自动触发。这个环节的意义在于它把审查者的工作从“核对代码是否规范”变成了“思考代码是否正确”。我见过很多团队没有这一层防线审查者每天花大量时间在评论里写“这里缺个空格”“这个变量名拼错了”这不仅浪费精力而且会让被审查者觉得审查者“挑刺”对审查文化的破坏性极大。3.2 提交切分从源头控制单次审查的规模前文提到审查规模控制在 200 行以内效果最佳。那么怎么保证每次提交都是小步提交呢这里有一个方法论层面的转变不要为了“完成一个功能”而提交而是为了“完成一个可审查的单元”而提交。举个例子你开发的是一个用户登录功能涉及数据库表结构、后端接口、前端页面三部分改动。如果你一刀切地把所有改动塞进一个 PR审查者就要同时切换数据库、后端、前端三个视角认知负担极大。更好的切法是把整个功能拆成三个 PR第一个只做数据库迁移脚本第二个做后端 API 和测试第三个做前端页面对接。每一个 PR 的审查者都能在自己的舒适区内专心地看一件事。有人说这样太慢了开发效率会下降。我的真实体验恰恰相反小步提交表面上增加了 PR 数量但每个 PR 的审查速度飞快整体等待时间反而缩短了。而且小步提交还有一个隐藏好处如果某个方案中途出了问题回滚的代价也小得多。3.3 审查分工谁来审、审什么、怎么算通过接下来是审查分工的设计。很多团队的问题是“所有人审所有东西”结果就是没有人真正对审查结果负责。我建议的模型是每个 PR 至少需要两类审查者——一个是熟悉这块代码的领域负责人负责技术正确性和设计合理性另一个可以是团队里的任意成员负责从新鲜视角补充观察。第二个角色的价值在于他们不了解历史包袱反而更容易发现“当局者迷”的问题。审查范围上我给自己定的检查顺序是固定的这样可以避免遗漏先看 PR 描述和设计意图确认改动方向是否有问题再看核心逻辑重点追踪数据流和状态变化然后看边界条件和异常处理接着看接口设计和命名是否清晰最后才扫一眼风格和文档至于“怎么算通过”我的标准是存在未解决的“必须修改”类评论时不允许合入。但“建议类”评论不阻塞合入作者可以选择后续跟进。这个区分很重要——它既保证了审查意见有分量又不至于让审查变成一个无底洞。建议类评论如果强制阻塞合入作者为了推进度就会开始无脑照单全收反而制造出更多不一致的风格问题。4. 实操过程手把手带你走一次完整的 Open Code Review理论部分讲完了接下来进入实操环节。我假设你用的是 GitHub 的 Pull Request 流程但同样的逻辑完全适用于 GitLab、Gitea 甚至 Gerrit。我会分五个步骤把一次完整代码审查的实操全过程拆解给你看每一步都附上我的实际经验和话术。4.1 第一步把 PR 描述当成“给审查者的使用说明书”我见过太多 PR 的描述栏是空白的或者只写了一句话“fix bug”。这种 PR 是对审查者时间的巨大浪费。审查者打开一个 PR首先要搞清楚的问题是“这个改动为什么存在”如果你不告诉他上下文他只能自己从代码里猜猜的过程既慢又容易误会。我给自己立的规矩是PR 描述至少包含四个部分——背景、改动内容、测试情况、风险点。背景要说清楚这个需求从哪来产品反馈、线上事故、还是自驱优化改动内容要列出核心文件分别做了什么测试情况要说清楚本地跑了哪些用例、手动验证了哪些场景风险点要主动承认哪些地方可能有隐患。别小看最后这一条主动说风险反而会增加审查者对你的信任。我举一个实际写过的 PR 描述示例背景用户反馈在弱网环境下上传大文件时进度条长时间卡在 90% 不更新。 改动重构了上传进度计算逻辑改为基于已确认字节数而非已发送字节数计算进度新增重试日志。 测试本地模拟弱网环境跑通了 1GB 文件上传新增了 3 个针对进度计算的单测。 风险进度回调频率提升需要关注对主线程的占用。这种描述写在前面审查者一上来就掌握了全部关键信息可以直接开始读代码而不是先花十分钟考古。4.2 第二步按逻辑分批提交而不是按时间堆提交很多开发者喜欢疯狂本地提交然后再 rebase 成一个大而全的 commit或者干脆把几十个小 commit 原封不动推上去。这两种做法都不利于审查。我的建议是把分支上的提交整理成有逻辑顺序的一组提交每个提交完成一个独立的、可审查的逻辑单元。比如你做了一个功能涉及基础工具函数、业务逻辑、测试用例那就拆成三个提交第一个提交加工具函数第二个提交写业务逻辑第三个提交补测试。审查者可以按提交顺序逐个 review每一步的认知负担都大幅降低。这个习惯用 Git 操作实现起来很简单开发过程中随便你提交提交完之后用git rebase -i对提交进行整理把相关改动合并、重新排序必要时用git commit --amend补充遗漏的修改。整理完再 push 到远端发起 PR。我在工作里经常看到有人嫌 rebase 麻烦宁可推一堆杂乱提交上去到最后审查者根本不知道从哪看起只能回一句“能 squash 一下吗”一来一回又浪费半天。4.3 第三步自测与自审做自己的第一个审查者提交 PR 之前我强烈建议你做一轮自审。不要以为自己是作者就看不出问题恰恰相反作者自审的效率往往比审查者更高因为你对代码的上下文理解最充分。自审的具体做法是在提交之前用 Git 的 diff 功能把自己这次改动的完整 diff 从头到尾看一遍。注意是在提交之前不是提交之后。这个习惯叫作“diff review”diff 审查是很多资深工程师秘而不宣的武器。当你以 diff 的形式回看自己的改动时你会惊讶地发现自己能找出不少问题——多余的调试日志、忘记删除的注释、不一致的命名、遗漏的边界情况。我在自审时还会问自己三个问题这段代码一个月后我还能看懂吗如果它出了 bug日志能不能帮我定位问题有没有更简单的方式实现同样的功能这三个问题看起来朴素但在自审阶段特别有效。尤其是第二个问题——日志是否完善——我见过太多代码功能实现了但一旦线上出问题日志里全是“error”两个字根本定位不到具体是哪一行出的问题。4.4 第四步提交后的讨论与迭代用对交流方式让审查高效推进PR 提交上去之后审查者和作者之间的讨论就开始了。这个环节里交流方式直接决定审查是变成高价值的协作还是变成火药味十足的拉锯战。对于审查者我给几个具体的建议。第一审查意见写成“提问”而不是“断言”。不是“这里写得不对”而是“这里的边界情况是不是没有处理如果用户传入空字符串会怎样”提问的姿态给了作者思考的空间也给自己留了余地——万一你没看懂上下文对方解释之后你就明白了不必拉下脸承认错误。第二给建议时最好带上具体方案。不要只说“这个函数太复杂了”要说“这个函数拆成两个会不会更好比如解析部分和处理部分分开”。第三区分“必须修改”和“可以考虑”。在 GitHub 上我习惯用nit:小问题前缀标注非阻塞意见用普通评论表达核心问题。对于被审查的一方最重要的一条是不要把审查意见当成对你个人的攻击。听到反对意见时先默认对方是好意的冷静评估他的建议是否有道理。如果觉得建议不合理用事实和数据去说明而不是用情绪去反驳。“这块代码之所以这么写是因为要兼容旧的接口格式你建议的写法会导致老客户端挂掉”——这种有理有据的回应任何理性的审查者都会接受。4.5 第五步合入与复盘把一次审查的价值延伸到下一次PR 合入之后代码审查并没有真正结束。我强烈建议团队定期做一次轻量级的审查复盘频率不需要高每两周或每月一次即可。复盘的内容很简单抽查几单合入的 PR看看当时审查意见的质量如何有没有漏掉关键问题作者的回应是否合理整体周期是否过长。这听起来有点像额外的工作量但它的价值非常大。审查是一件短期内很难看到反馈的事情——你很难说清楚“因为上次审查拦下了一个 Bug所以省了多少线上的事故成本”。复盘就是给审查建立反馈回路的手段让你能持续地调整和优化审查方式。另外一个我特别推荐的合入后的做法主动去关注你审查过的代码上线之后的表现。如果出了问题回头看一下当时为什么没发现如果没有问题想一下是运气好还是真的覆盖到了关键点。这个习惯比较反人性——人都喜欢完成一件事之后立刻把注意力切换到下一件事上。但我这几年能感觉到自己在审查上的“嗅觉”明显比前几年敏锐很大程度上就是靠这种合入后的回看一点点喂出来的。5. 常见问题与排查技巧实录最后这部分我整理了一些在实际推动代码审查过程中非常容易遇到的典型问题以及我验证过有效的应对方法。这些问题覆盖面比较广从流程设计到人情世故都有涉猎你可以当成一份速查手册来用。5.1 审查流于形式评论永远只有“LGTM”问团队里的代码审查已经变成纯粹的流程装饰品审查者基本不看代码评论永远只有一句“LGTM”怎么破这是我在中小型团队里见过最多的问题。背后通常有两个原因一是大家业务压力太大确实没有时间认真看二是害怕得罪人不想提反对意见。针对前者解决办法是前文反复强调的——缩小单次审查规模确保审查者的负担在可承受范围内。针对后者需要从文化层面入手团队负责人要公开给审查意见“撑腰”让大家意识到提意见不是找麻烦而是对团队负责。如果以上两种手段都用了审查质量还是没有改善那我就会建议引入随机抽查机制每周由技术负责人随机挑出几个已合入的 PR当着全组的面拆解其中的问题和亮点让“没认真审”的人无处遁形。这招见效很快但要注意分寸重点是教育而非惩罚。5.2 一次审查的改动量太大根本看不进去问接手了一个改动 3000 行的 PR时间紧任务重硬着头皮看根本看不进去怎么处理这种情况下不要强迫自己硬看。先把 PR 的审查暂停然后去和作者沟通请他优先把改动按照逻辑拆成多个小 PR 或以“一次一个提交”的顺序推给你审查。200~300 行以内的拆分标准作者只要愿意配合通常花不了太多时间就能完成。如果这个 PR 实在不方便拆比如整体重构场景那就换另一个策略不要试图从头到尾逐行读完。先用 20 分钟时间快速浏览整体 diff找出架构层面的问题然后挑核心模块深入读细节其他模块只看关键分支和异常处理。与其低质量地看完 3000 行不如有策略地精读 500 行核心代码。审查的目标是发现问题不是像读书一样从头翻到尾。5.3 被审查者觉得被冒犯沟通逐渐变成对抗问审查意见写得太直接被审查者觉得被针对两个人关系闹得不太愉快怎么修复这种情况处理起来需要一点软技巧。首先是尽量把沟通放到公开场合而不是私聊里面夹杂个人情绪公开的代码审查记录会迫使双方保持基本的理性和礼貌。其次是学会使用“我”开头的表达方式“我理解这个地方可能有特殊背景但我看的时候有点困惑……”这种表达方式不是在否定对方而是在坦诚自己的理解局限对抗性会大幅降低。再分享一个挽救关系的技巧在同一个 PR 里既要提问题也要肯定亮点。我看到很多人只关注代码的问题对写得好的部分一言不发。但实际上“这个函数的名字起得真好一眼就能看懂用途”或者“这个测试用例的覆盖思路很有启发”这样的正面反馈才是审查关系里最好的润滑剂。别吝啬真诚的夸奖它不会让你显得不专业反而会让你显得更可信。5.4 紧急修复绕过审查“欠债”越欠越多问线上出故障了时间紧迫开发者直接绕过审查合入修复代码导致“技术债”越积越多。这种紧急情况到底该不该走代码审查我的答案是紧急修复可以不走完整的审查流程但必须走“最小的安全流程”。最基本的底线是修复后再补一次审查简称“事后审查”。别小看这个事后操作它的价值在于给紧急修复的代码建立一个反馈机制。线上紧急修复往往是在极大压力下写的质量隐患本来就高如果事后也不补审那些问题就会永远沉睡在代码里。具体来说我的习惯是紧急修复合入后要求作者在 24 小时内补一个 PR把当时的修复代码作为 diff 提交然后由团队内指定的人进行一次正式审查。这时候审查的目的不是“改代码”修复已经上线了而是“复盘问题并在后续迭代中修正”。经过这样的机制紧急修复才会变成一次学习机会而不是一次质量上的污染。5.5 自动化工具到底能不能替代人工审查问团队上了很多自动化质量关卡静态检查、单元测试覆盖率、CI 都做得很好是不是就可以不用人工审查了这个问题我几乎每年都会遇到。我的答案永远是不行。自动化工具是“守门员”不是“教练”。守门员能挡住飞向球门的球但无法教你怎么打出更好的进攻配合。静态检查能抓住“空指针解引用”这种确定性错误但抓不住“这个模块的职责划分有问题将来扩展性会很差”这类设计层面的问题。单元测试能验证函数的行为符合预期但验证不了“这个函数的接口设计对调用者是否友好”。人工审查和自动化工具有一个本质区别自动化工具是在“验证”人工审查是在“理解”。前者基于规则后者基于判断。规则可以覆盖历史问题但无法预判新问题判断虽然没有固定模式却能应对无限变化的新场景。我理解的理想状态是自动化工具把所有客观标准把关好让人工审查者有富余的精力专注于设计和逻辑层面的深度思考。两者缺一不可而且它们的分工要清晰不能混着来。写在最后的几句心里话回头看看这几年代码审查对我来说已经从最初“不得不做的流程”慢慢变成了“能够提升自己视野的窗口”。每次认真审查别人的代码我都会发现一些自己写代码时也会犯的错误也会从别人的巧妙设计里偷学到不少招法。同样每次收到高质量的审查意见即便当下有些脸红冷静下来都会庆幸——好在这些坑在上线之前就被发现了。如果你打算在团队里认真推行代码审查我最后的建议是不要用高压手段逼大家。先从工具链的自动化做起把基础防线的成本降下来再靠小步提交把单次审查的负担降到合理范围最后用正面反馈和复盘机制慢慢建立起健康的审查文化。这个过程不会一蹴而就但一旦建立起来它对团队代码质量、知识沉淀和新人成长的回报会远超你的预期。