1. 为什么绝大多数代码审查都是走过场
先说个技术圈的老问题：code review这个词几乎每个团队都在提，每个技术负责人都在强调“一定要做”，可真到了落地的时候，大多数团队的评审流程都停留在“看完给个 LGTM”的状态。我待过几个不同规模的技术团队，也帮朋友看过他们公司的代码评审流程，说实话——真正把 code review 做出价值的团队，十个里也就两三个。
open-code-review这个方向，我理解的不只是一个开源工具，而是一整套“如何把代码审查做透明、做高效、做可持续”的方法论。它的核心含义有两层：第一层是工具和流程层面上的“开放”，从需求到提交再到评审意见，全程可追溯、可讨论、可改进；第二层是心态和协作层面上的“开放”，团队成员愿意认真看别人的代码，也愿意接受别人认真看自己的代码。
这套东西能解决的问题其实很具体：因为评审流程不透明，所以审查意见经常被当成“找茬”；因为工具链落后，所以审查过程变成在聊天软件里来回贴代码片段；因为缺乏统一的评判标准，所以同一个改动在不同审查者那里可能得到完全相反的结论。如果你正在做团队的技术管理、负责搭建研发流程，或者单纯想让自己的开源项目接受外部贡献者的代码时更有条理，这篇文章应该能给你一套可以直接抄作业的方案。
我接下来要讲的，不是那种“建议大家多沟通、多协作”的虚话，而是从分支策略、工具选型、MR/PR 写法、审查清单、自动化门禁到人性化沟通的完整实操路径。
2. 先把“开放”这件事落到流程设计上
2.1 特性分支模型：别再用主干直推当评审入口
很多小团队一开始并没有代码评审的习惯，大家都是在主干分支上直接开发、直接提交。这种模式在一个人写一个模块的时候勉强能跑，可一旦两个人改了同一个文件，或者某个功能上线后出了线上问题需要回滚，场面就很容易失控。
要做开放式的 code review，第一步永远是把“提交代码”这个动作和“合并代码”这个动作拆开。主流的做法是短特性分支模型：开发者在本地从最新主分支拉一个 feature 分支，所有改动都提交到这个分支上，确认功能完成后把分支推到远端，通过合并请求（MR）或拉取请求（PR）发起评审，评审通过、自动化检查通过之后才允许合并回主分支。
这套模型听起来简单，但多数团队卡在“分支保护”这一步没做。正确的做法是在 Git 服务端开启主分支保护规则，禁止任何人直接向主分支 push。开发者唯一的提交通路就是 MR/PR，这样代码评审就从“可选项”变成了“必选项”。
以 Gitea 或 GitLab 自建服务为例，保护规则里我会建议这样设置：
- 禁止直接 push 到主分支（勾选拒绝强制推送和普通推送）
- 至少需要 1 个或 2 个审查者的批准（视项目关键程度调整）
- 新提交推上来之后，旧的批准状态要自动失效
- 合并前要求所有自动化检查通过
我之前帮一个创业公司搭过这套流程，刚上线那天一个后端同事抱怨说“太麻烦了，改个变量名都要走一遍流程”。两周之后再问他，他自己也承认，有了流程约束之后，他基本没再因为改错公共方法而被线上问题折腾过。原因是每次提交都有人把关，等于你自己前面多了一层测试。
2.2 自建还是托管：工具选型要盯住“可追溯性”
工具层面的选择，常见的无非三条路：GitHub 公共托管、自建 GitLab 或 Gitea、还有纯本地的 Gerrit。每条路的取舍很不一样。
GitHub胜在生态最丰富，外部协作最方便，如果你做的是开源项目，那基本不存在第二个选择。缺点是国内访问稳定性需要自己评估，私有仓库的合规审查也需要额外考虑。
GitLab / Gitea 自建的好处是数据完全在自己手里，可以深度定制评审流程和自动化脚本，权限管理也更细。Gitea 非常轻量，我甚至在一台 2 核 4G 的小机器上跑过，团队几十个人的日常评审完全没问题。GitLab 功能更全，但资源占用高一些，维护成本也高一些。
Gerrit是另外一个极端，它把“评审”这个动作嵌入到 push 协议里，强调逐 commit 评审，适合特别严肃的大型项目或嵌入式/内核开发场景。但对大多数业务团队来说，它的心智负担偏高，普遍反馈不太好用。
我自己的选型策略是：开源项目直接走 GitHub；企业内部团队，如果规模在 50 人以内，Gitea 足够；如果有复杂的 CI/CD 集成、需要原生的 DevOps 面板，GitLab 更合适。这里说的“够用”是指：MR/PR 的讨论区、多轮修改记录、审查人批准状态、自动化检查状态回写，这些功能必须原生可用。
工具选型还有一个特别容易忽视的点——数据迁移成本。我建议团队无论选哪个，都要把“仓库数据能够定期备份、能够迁移”作为前提。代码是资产，评审记录也是资产，丢了哪个都是事故。
2.3 评审范围与 MR 颗粒度：多大的改动才算合理
流程搭好之后，第一个真正影响 code review 质量的现实问题就是 MR 的颗粒度。我看到过一千多行改动的 MR，也看到过只有一行改动的 MR。前者会让审查者直接摆烂，后者倒是轻松，但对项目整体演进来说效率并不高。
什么样的 MR 大小最合适？我个人的经验值是：一个 MR 对应一个完整的小功能点或一个明确的缺陷修复，改动量尽量控制在 200 到 400 行以内，涉及的文件不超过 10 个。这个数字不是拍脑袋来的——当改动量超过某个阈值时，审查者注意力会快速衰减，漏掉的缺陷会明显上升。
拆 MR 还有一个具体的操作技巧：不要等代码全部写完再拆，而是在开发过程中就按提交节点去拆。比如一个登录功能，我先完成数据库表和实体类，提交一次；再完成登录接口和参数校验，提交一次；最后写单测和接口文档，再提交一次。这样每个提交本身是自洽的，审查者可以按提交顺序逐个看，理解成本低很多。
这里有一个可以“抄作业”的心得：MR 描述里第一句话就写清楚“这个 MR 做了什么、为什么做、不做什么”。我见过太多人只写一句“fix bug”就把 MR 丢出来了，审查者左看右看看不出到底为什么改、改了之后会不会影响其他地方。描述写得好，评审效率至少提升一半。
3. 让自动化帮你守住基础底线
3.1 静态检查与格式化：别让人去当格式检查器
代码审查里最浪费生命的一类事，就是审查者在一堆格式问题上花时间：缩进不对、命名不规范、import 顺序乱、多余的逗号……这类问题完全不应该出现在人工评审环节，因为自动化工具有非常成熟而且免费的方案。
以我常用的 Go 项目为例，我会在 CI 里加入gofmt检查、go vet静态分析，以及golangci-lint的常用规则集。前端项目则可以使用 ESLint + Prettier，Python 项目可以用 Ruff 或 Black。关键是把这些工具的执行结果接入到 MR 的状态检查里，让不合规的代码根本无法合并进主分支。
有人可能会问，那是不是有了这些工具，人工评审就完全不用看代码风格了？并不完全是。自动化解决的是“全量、无情绪、可执行”的规则校验，但风格背后的合理性仍然需要人来判断。比如一个函数明明可以拆成三个更内聚的函数，或者一段逻辑本可以复用现成的基础库，这类问题工具是看不出来的。正确的关系是：自动化做底线防守，人工做质量上限提升。
3.2 覆盖率与单测：门禁要有，但不能盲从
测试覆盖率这个指标，在 code review 里是最容易被滥用的。有的团队硬性规定“覆盖率必须到 80% 才能合并”，结果开发者的第一反应不是思考测试怎么写得更好，而是用一堆无断言的假测试去凑覆盖率。我自己之前就见过一个项目，覆盖率报告显示 85%，但核心的支付回调逻辑连一个真实的异常分支都没测到。
覆盖率门禁的正确用法是“分模块分策略”：核心业务模块的增量代码覆盖率要严格卡，比如 70% 到 80%；工具类、配置类、常量类则不需要硬性要求。还有一点很重要，覆盖率门禁应该关注的是“本次 MR 新增代码的覆盖率”，而不是全项目的历史累计覆盖率。
在审查清单里，我会要求自己重点看三个点：测试有没有覆盖正常路径之外的分支？有没有覆盖错误处理路径？测试断言是“验证了行为”还是只是“跑了一遍不报错”？第三个问题尤其误导人，很多所谓测试用例连 assert 都没有，跑过就是绿灯，这种不如不写。
3.3 门禁顺序的讲究：先快后慢，别让开发干等
设计 CI 流程的时候，顺序也有技巧。常见的问题是：团队把所有检查都串在同一个流水线里，一次改动推上去要先跑 20 分钟的完整构建才能看到 lint 结果，改一行注释也得等半天。
更合理的编排思路是分阶段跑：第一步只跑代码格式化检查和基础语法检查，这类任务几十秒内出结果；第二步跑单元测试和覆盖率收集；第三步跑构建和集成测试。每一步失败就立即中止，让开发者第一时间看到最直接的反馈。这样大部分日常改动都能在 5 分钟内得到结果。
我还习惯在 CI 里加一个“评论机器人”或“状态标记”来汇总检查结果，不过实现方式要看团队基础，不必强求。
4. 人工评审时到底在看什么
4.1 先看需求再谈实现：别陷入局部最优
我见过很多审查者刚拿到 MR 就盯着某一个 for 循环的效率问题开始长篇大论，结果整个功能的需求逻辑都没理清楚。这里很容易犯的毛病是把注意力全部放在“代码怎么写得更优雅”上，而忘了问一个更前置的问题：“这个代码解决的需求到底成不成立？”
我在评审时养成了一个习惯：拿到 MR 之后，先不急着看 diff，而是先看描述和关联的需求单或 issue。先把“为什么要改”搞清楚，再对照实现去看“是不是这么改的”。如果需求描述和实现明显对不上，那就不用往下看了，这个 MR 可以整体打回。
具体操作上，可以要求团队在 MR 模板里设一个必填字段“需求链接/问题链接”，把业务背景和工作项关联起来。强制填写之后，审查者甚至不需要熟悉这个功能模块，就能通过需求单快速建立上下文。
4.2 函数级别审查的几个关注点
逐行看代码的时候，我会比较关注这几个点：
看函数是否做了太多事。一个函数里既有参数解析、又有业务判断、还有数据落库和日志埋点，那这个函数基本不具备可测试性。识别方法很简单：如果这个函数很难写单测，那大概率职责过重。
看空值和错误的处理路径是否完整。实践中最常见的崩溃来源不是复杂算法，而是拿到 null 之后直接调用方法，或者异常被吞掉导致后续状态不一致。
看命名是否表意。这是我个人很坚持的一点。变量名、函数名是代码自文档化的基础，与其写一堆注释来解释“这个变量是干嘛的”，不如把变量名改成一看就懂的样子。审查里遇到命名抽象、含义不明的代码，我一定会提出来。这不是吹毛求疵，而是这类代码在三个月后就会变成团队里“看不懂，别动”的定时炸弹。
看是否复制粘贴了代码。如果一个逻辑片段与另一个文件里已有的实现高度相似，审查者应当主动指出，并推动提取公共函数。一次两次的复制看似省事，长期看却是维护成本翻倍的开始。
4.3 安全与性能的快速排查：不需要是安全专家也能发现问题
很多开发者觉得安全性审查是安全团队的事，普通业务开发不用管。这个想法在小型和中型团队里是很危险的——大多数团队根本没有专职安全人员，安全防线就是靠 code review 一关一关过。
业务开发在评审时，至少可以关注几个明显的安全隐患：用户输入有没有做合法性校验；SQL 查询是参数化还是字符串拼接；敏感数据（密码、token）有没有被记录到日志里；接口有没有明显的越权问题；文件上传有没有限制类型和大小。
性能方面也一样，大部分代码不会有高并发之下的性能问题，但结构性问题值得关注：有没有在循环里去查数据库或者调用外部接口；有没有明显多余的全表数据加载；有没有在前端包里打包了过大的依赖而且完全没用到。
这些问题不需要多高深的知识，靠常识和几条固定套路就能发现，但很多开发者在评审时压根没有这个意识。我在自己的团队里会把这类问题做成清单，强制要求每个 MR 的描述里标注“涉及安全：是/否”，如果涉及就要求补齐安全检查记录。这个简单的动作，就能让大家都下意识地多想一步。
4.4 测试质量的评审：别只看覆盖率数字
前面提到覆盖率不能作为唯一标准，那人工评审测试代码时到底看什么？我一般关注三个层次。
第一个层次是断言质量。测试里有没有真正校验期望值？还是只是调用了一下函数什么都没检查？第二层次是分支覆盖。表面看一个函数测了，但只测了成功路径，异常分支全部没走到。第三层次是场景相关性。测试用例是不是真实模拟了线上会遇到的情况，还是制造了一个理想环境自欺欺人。
举个例子：一个支付接口的测试，如果只测了“输入合法参数返回成功”，却完全没测试“参数不合法返回 400”“余额不足返回错误码”“重复提交幂等拦截”，那这个单测对业务来说几乎没有任何保护价值。审查时我会明确把这些场景缺失指出来，并且会问一个问题：“如果这段代码出了 bug，你写的这些测试能拦住吗？”这个灵魂拷问，特别能筛选出低质量的测试。
5. 开放审查的人际协作与效率陷阱
5.1 从“代码写得不行”到“这个实现的风险点在于……”：重构沟通话术
代码审查实际上是一门沟通技术，很多团队的 code review 流程本身没问题，但毁在沟通方式上。我见过不少开发者一听到别人的审查意见就本能地防御：“为什么一定要这么改？”“我这么写也没什么问题吧？”双方僵持不下，最后要么是主导者强行推进，要么是意见被搁置，整体效率都很低。
更有效的方式，是把审查里的一切讨论都引导到“具体问题、风险、方案”这个框架里，而不是对人下判断。与其说“你这里不能用 map，性能不行”，不如说“这个 map 在循环里会被反复初始化，当前数据量小看不出问题，如果后续数据量上来，这里会成为热点，要不要考虑提到循环外面初始化？”
我在团队里还推行过一个具体的规则：审查意见分为 BUG、IMPROVE、NIT、SUGGESTION 四级。BUG 会阻塞合并，IMPROVE 建议修改，NIT 是风格细节，SUGGESTION 是不需要本次处理、记录到 backlogs 的点。这个分级机制一建立，讨论立刻理性了很多——至少没人会花十分钟争一个 NIT 该不该改。
与之配套的操作规范是每一个审查意见必须给出“为什么”，不能说“这里不对”就不管了。我一般要求意见写成“问题描述 + 影响范围 + 修改建议”的三段式，这样提交者不需要猜审查者到底想要什么。
5.2 响应效率：同步评审、异步评审和结对评审的取舍
评审的响应速度是另一个影响体验的点。如果提交 MR 之后三天没人理，那这个流程注定会被开发者用各种方式绕过。常见的评审模式有三种。
传统异步评审（也就是比较常见的“评论+等待”）适合大多数日常变更，灵活但容易拖延。我建议团队约定一个明确的 SLA：工作时间内，首次评审意见在 4 小时内给出；如果评审者确实忙，也要先点个“评论”说明什么时间看。这个约定不需要复杂的工具支持，只需要口头和书面都写清楚。
同步评审适合复杂的大变更，大家约一个会议室或在线音视频，共享屏幕，由提交者从头到尾讲一遍改动，审查者随时打断提问。通常一次同步评审能解决很多异步评审里来回打字扯皮的问题，效率非常高。我曾经参与过一个支付模块的核心变更评审，是一个 600 多行的 MR，异步评审两天了还没结束，后来拉了个会，40 分钟就把所有问题梳理完了。
结对评审是另一种形式，两个开发者坐一起实时看代码、实时修改，节奏更快。它的缺点是人力占用高，不适合所有场景，但在处理复杂算法、核心架构调整时非常值得用。
5.3 防止“LGTM 依赖症”：让审查真正发生
一个特别常见的失败模式是“LGTM 依赖症”——所有 MR 看起来都有人批准了，但实际上每个审查者都没怎么认真看过代码。出现这种情况，通常是大家觉得“别人的代码我不好说太多”“反正 CI 过了”“另一个审查者已经看过了”。
为了打破这种集体敷衍，我在团队里做过几件事。第一件事是强制在 MR 里写清楚“测试验证步骤”，让审查者能够照着步骤本地复现，而不是纯靠读代码脑补。第二件事是定期更换不同功能模块的审查人，让每次审视都有新鲜视角。第三件事是明确规定批准的含义——点击 Approve 意味着“我认真读过代码，验证过测试，认可这个实现”，而不是“看起来问题不大”。这个定义明确之后，滥批的现象会明显减少。
另外一个经常被忽视的问题是要建立“二次审查”机制：当第一个审查者提出比较大的修改意见之后，提交者修改完再推上来，第二个审查者不能只看看最新回复就通过，而要关注变更引入了哪些新的影响。Git 服务端的“新提交使旧批准失效”功能正是为此设计的，一定要开启。
6. 我在真实项目里踩过的几个坑
6.1 大爆炸式 MR 让评审名存实亡
几年前我参与过一个数据迁移项目，当时的代码组织方式是“憋大招”：开发一个月，最后一口气提交了一个 8000 多行的 MR。整个团队评审了整整一周，谁也说不清具体每处改动的来龙去脉。结果上了测试环境之后，连续崩了三次，每次定位问题都要在八千行代码里重新过一遍逻辑。
那次经历的教训是刻骨铭心的：代码审查的前提永远是“可被审查的粒度”。在那之后我给自己定了一条铁律，大功能必须按模块、按阶段拆 MR，每次改动不超过一个完整的可交付单元。这个习惯后来也帮我避了很多雷——改动小，出了问题定位就快。
6.2 自动化用例拦截了本应被人工发现的逻辑错误
有一段时间我们特别信任自动化测试，CI 全绿就觉得代码质量没问题。结果有一次订单超时关单的功能上线后，用户反馈部分订单被提前关闭。排查后发现是时区处理的问题——服务器用 UTC 存储时间，但业务比较时用了本地时间，导致边界判断偏移了一个小时。这个逻辑错误完全逃过了所有自动化测试，因为没有测试用例专门覆盖时区切换场景。
真正发现问题的是上线后的人工回归评审。那次之后我调整了工作方式：自动化测试证明的是“代码按我写的逻辑运行”，人工评审验证的是“我写的逻辑是否符合真实业务”，两者不可互相替代。再安全、再充分的自动化，也不能替代一个认真思考过业务场景的资深工程师的眼睛。
6.3 对新人过度纠正，差点把代码评审变成了心理负担
这是我的亲身体会。有段时间团队新人比较多，我在评审时特别较真，心里想的是“严格一点是对他们负责”，结果有好几次新人改了一版又一版，依然无法通过评审。直到其中一个人私下跟我说：“每次提 MR 都很有压力，感觉做这个项目就是不断被挑毛病。”
我这才意识到，code review 如果只剩挑错，就会摧毁团队的心理安全感。后来调整了做法：对新人提交的 MR，重点只抓 BUG 和影响明确的改进项，NIT 和风格类的意见合并成一轮给，并且每轮评审先写一句“这次改动里做得好的部分”。这个调整让新人的接收度明显好了不少，节奏也理顺了。
6.4 审查不是终点，沉淀经验才是真正的资产
最后聊一个容易被忽略的点：审查产生的讨论记录和问题类型，本身就是团队最宝贵的过程资产。可惜绝大多数团队评审完就散了，既没有统计问题类型，也没有总结改进项。我会建议每个季度做一次 code review 复盘，看看上季度最频繁出现的问题类别是什么：是需求理解偏差、边界条件遗漏，还是代码结构混乱？这些问题定向解决之后，下季度的评审效率和代码质量都会有明显的提升。
代码审查这件事，说到底不是靠某个工具或某个流程就能做好的，它需要一套“工具 + 规则 + 文化”的组合，也需要持续的跟进和调整。从我自己的经验来看，只要愿意先把这个流程搭建起来、跑顺，代码质量、团队协作效率、新人培养速度都会是肉眼可见的改善。希望这篇文章能给你一些可以落地的启发，直接从下一个 MR 开始试试。
