代码审查这件事，团队里的态度往往两极分化：有人觉得是走过场的仪式，有人觉得是最后一道保命防线。我属于后者，但前提是——审查的方式要对。多年前我也在"码了1000行，review 5分钟"的流程里难受过，后来折腾了很长一段时间，才把一套真正开放、可落地的 code review 流程慢慢打磨出来。今天聊的这套实践，不绑定任何商业工具，也不依赖某种"银弹"平台，核心是把代码审查变成一个团队都能参与、敢说话、真能发现问题的开放过程。无论你是十人小团队还是几十人的研发组，这篇文章里的思路、清单和排查技巧都可以直接参考。
1. 代码审查到底在解决什么问题
1.1 一个人写代码，一群人看代码的意义
很多开发新手会问一个很实在的问题：我自己写的代码，本地测试都过了，功能也验证了，为什么还要让别人来 review？这个问题背后的潜台词是"代码审查是为了挑错"，但这个认知其实只对了一小半。
代码审查更本质的作用是打破"信息孤岛"。你在自己分支上写了三天的逻辑，别人完全不知道你踩了哪些坑、做了哪些假设、改动了哪些公共接口。review 不是重新做一遍测试，而是让团队里至少另外一个人理解"这段代码为什么长这样"。这个理解过程本身，就是在为项目积累集体记忆。
我见过很多项目出问题，不是挂在复杂的业务逻辑上，而是挂在某个"当时没人知道这里有个约定"的细节上。举个实际例子：项目里有个工具函数叫formatDate，第一版实现只处理了YYYY-MM-DD格式。后来有人为了兼容某个第三方接口，悄悄加了第二个参数format，但既没改测试也没更新注释。三个月后，另一个同事需要格式化时间，看了一眼函数名的签名，根本没注意到那个可选的第二参数，于是自己又写了一个formatDateV2。这种重复和分裂，靠单测是没法完全避免的，只有靠持续、开放、有人真正在读别人代码的 review 过程才能拦截。
1.2 真正有价值的审查目标排序
我在带团队和做技术咨询的过程中，逐渐把代码审查的"目标优先级"梳理成了下面这张表。这个排序非常重要，因为如果你把审查重点放错了位置，效率会非常低。
| 优先级 | 审查目标 | 典型问题示例 | 为什么重要 |
|---|---|---|---|
| P0 | 正确性缺陷 | 并发条件、空指针、事务未回滚 | 上线后会直接造成故障或资损 |
| P1 | 可维护性问题 | 命名混乱、函数过长、重复代码 | 决定三个月后改需求的人会不会骂人 |
| P2 | 性能隐患 | 循环内查询数据库、N+1 问题 | 数据量小的时候没事，量一大就爆 |
| P3 | 风格与偏好 | 缩进、括号位置、变量命名偏好 | 有规范就遵守，没规范不值得争论 |
这套优先级意味着，你在 review 别人的代码时，不应该一上来就揪着"这里少了个空行"不放。真正有经验的审查者会先看整体逻辑和边界条件，把时间和注意力花在最容易出问题的部分。
我自己刚开始做审查时也犯过"鸡蛋里挑骨头"的毛病，把大量注意力放在代码风格上，结果作者不太开心，真正关键的事务问题反而没看仔细。后来改变了策略：风格问题只在有明显的、团队约定俗成的规范时才提，正确性和可维护性问题才是评论的主体。这个转变基本重构了我个人 review 的口碑。
1.3 开放心态是代码审查的前置条件
"open-code-review"里的 open，我认为更多指的是一种心态和流程开放的姿态，而不是特指某种开源工具。
有不少团队把代码审查做成了"大佬审批"模式：只有技术负责人或者资深工程师才有资格 merge，其他人提交之后只能等。这种模式有两个明显的副作用。一是审查瓶颈严重，所有人的代码都堵在一个人那里，这一环的效率直接决定整个团队的交付速度；二是团队其他人的责任感会下降，反正有"大佬"兜底，我提交的时候不用太仔细。
真正开放的做法是"人人都能 review，人人都在 review"。初级工程师也可以对别人的代码提出疑问——他们可能不熟悉某个业务背景，但恰恰是这种"不熟悉"，让他们有可能发现文档缺失、接口设计反直觉、日志信息难以理解等资深工程师已经"看习惯"了的问题。代码审查不该是一言堂，它应该是一个团队层面的讨论场。
2. 核心审查模式与适用场景
2.1 异步审查：主流且高效的起点
目前最主流的代码审查模式是异步审查，也就是通过 Git 平台的 Merge Request（或者 Pull Request）功能，提交代码后等待其他人评论，整个过程不要求所有人同步在线。
异步审查最大的好处是灵活。审查者可以在自己方便的时间段打开 diff，逐行查看；作者也可以在不同步打断开发节奏的前提下收集到反馈。对于分布式团队、跨时区协作或者需要大块时间专注开发的情况，异步审查几乎是唯一可行的方案。
但异步审查有一个天然弱点：上下文丢失。reviewer 看到的是一堆 diff，但看不到作者在编程时的完整思路、设计取舍和业务背景。如果相关描述写得不够清楚，reviewer 就只能靠猜。这也是为什么我会特别强调 MR/PR 的描述要写得像一个微型技术方案，而不是只写一句"完成了XX功能"。
2.2 同步走查：复杂模块和关键路径的必要补充
对于复杂的核心模块、大型重构、或者涉及多个系统联调的关键改动，纯异步审查往往不够。我的经验是，这类改动即使异步 review 过一轮，仍然有必要拉一个短会做同步走查。
同步走查的具体形式可以很轻量：作者打开 diff 或者本地代码，投屏共享，从头到尾讲一遍自己改了哪些地方、为什么这么改、哪里自己拿不准。其他参与者随时打断提问。
这种模式的价值在于"即时澄清"。在异步审查中，一个疑问可能要等作者几个小时后看到评论才能解答，而且很容易出现"评论聊了三轮还没对齐"的低效情况。在同步走查里，一个疑问 30 秒内就能得到答复，四五个人的脑力可以同时聚焦在一个复杂设计上，效果非常明显。
需要特别说明的是，同步走查的时长需要严格控制。我实践下来，一次走查最好不要超过 45 分钟，人数控制在 3 到 6 人比较合适。人太多容易变成讨论会，人太少又起不到"多角度审视"的作用。超过 45 分钟还是讲不完，说明这次改动的粒度太大了，应该拆成多个更小的 PR 再来走查。
2.3 不同规模的团队怎么选模式
我经常被问到的一个问题是：我们团队就五个人，还需要搞代码审查吗？或者反过来：我们团队一百多人，代码审查是不是应该用很重的流程？
先说小团队。五个人的团队反而应该坚持做 review，因为人越少，每个人的代码被其他人理解的机会就越少，一旦有人请假或者离职，某个模块可能就完全没人能接手了。小团队不需要很重的流程，一个简单的约定就可以：所有 MR 必须至少有一个非作者的同事 approve 后才能 merge。
再说大团队。大团队的问题不是"要不要 review"，而是"怎么让 review 不流于形式"。大团队容易出现的现象是：每个 MR 随便分配给一个"看起来相关"的人，reviewer 不做深度理解，只回复一个 LGTM。这种情况下，需要把审查责任和模块归属挂钩。比如，谁负责这个模块的维护，谁就必须对相关 MR 的 review 质量负责。
下面这张表是我经常在团队里做分享时用的，整理了不同规模团队比较合适的审查模式组合。
| 团队规模 | 推荐模式 | 关键约定 |
|---|---|---|
| 2-5人 | 异步审查 + 关键模块同步走查 | 所有 MR 至少一人 approve 才能合并 |
| 6-20人 | 异步审查为主，按模块指定 reviewer | 核心模块必须由模块 owner 参与审查 |
| 20人以上 | 异步审查 + 定期同步走查 + 自动化检查 | 设置清晰的审查人指派规则，避免随机分配 |
3. 实操过程：从提交代码到完成审查的完整闭环
3.1 提交阶段的三个关键准备
很多人把 code review 当作 merge 之前的"关卡"，但实际上提交阶段做的好不好，直接决定 review 的质量和效率。我自己的项目里，对"提交准备"有三个硬性要求。
第一，PR 的粒度要小。一个 PR 最好只做一件事，改动文件尽量控制在 10 到 15 个以内，改动行数尽量控制在 500 行以内。超过这个规模，reviewer 的注意力会明显下降。这一点在代码审查研究里有不少数据支撑：人的阅读带宽有限，单次 review 的 diff 过大时，漏检率会显著上升。把大改动拆成多个语义独立的小 PR，每个 PR 的 review 难度就会低很多。
第二，PR 描述必须写清楚"背景、方案、影响面、测试验证"四件事。我见过太多只写"fix bug"或"update"的 PR 描述，这种描述等于把背景调查工作完全丢给了 reviewer。
第三，在提交人自己创建 MR 之后、指定 reviewer 之前，至少要自己把 diff 完整看一遍。这一步看似多余，但实际上能拦截掉大量低级问题。很多时候，我们在 coding 时会"眼睛自动补全"，觉得自己写都写了还能有问题？但只要自己点开 diff 假装是另一个人来看，很快就能发现拼写错误、忘了删的调试日志、临时写死的分支判断等等问题。自己先检查一遍，既能减轻 reviewer 的负担，也能为自己建立"代码质量靠谱"的口碑。
3.2 审查清单：四个层次逐项检查
我自己在 review 时会按照"架构-逻辑-细节-测试"四个层次依次来看。这个顺序不是随便定的，是从"影响面最大"到"影响面最小"去排列的，可以保证在注意力最充沛的阶段处理最重要的问题。
架构层：这次改动跟现有模块的边界是否清晰？有没有把本不该耦合的东西耦合在一起？接口设计是否合理？改动是否引入了不必要的全局状态？这个层面的问题通常最难改，但如果发现了，价值也是最大的。
逻辑层：核心分支是否覆盖了所有边界情况？有没有并发安全问题？异常处理路径是否完备？事务边界是否正确？这个层面是 reviewer 的主战场，也是发现问题最多的地方。
细节层：命名是否清晰？有没有明显的资源泄漏？有没有不小心提交了调试代码、密钥、或者无关的格式化改动？这个层面不需要花太多时间，但要保持留意。
测试层：这次改动有没有对应的单测或集成测试？测试是不是只覆盖了"快乐路径"？有没有对边界条件和异常场景做验证？
这四层检查完，基本就完成了一次完整的 review。实际执行时，我会把评论按严重程度分成两类：一类是"必须修改后才能合并"的阻塞项，一类是"可以后续优化"的建议项，然后在评论标题里明确标注。这样作者就能很清楚地知道，哪些是硬要求，哪些是锦上添花。
3.3 评论话术与情绪管理
代码审查里，技术问题往往好解决，但"人的情绪"和"沟通方式"才是真正的深水区。我见过不止一次，因为 review 评论的语气问题，两个同事在评论区里吵得不可开交，最后技术问题没解决，关系还搞僵了。
我给自己定过几条评论纪律，在这里分享给各位。
- 对事不对人。把评论文本聚焦在"这段代码"和"这个场景"上，不要延伸评价作者的能力或态度。
- 先肯定再指出问题。如果某段代码写得确实漂亮，或者方案选得合理，不要吝啬一句肯定。这会让作者在心理上更容易接受后面的建议。
- 指出问题时尽量说"为什么"。单纯说"这样写不好"是没用的，要解释"在什么场景下这样写会出问题"。比如"这里如果用二分查找，数据量大的时候性能会更好"就比"这里性能有问题"有帮助得多。
- 建议尽量给出可选方案。能给出替代写法的，就给出参考代码；给不出完整方案的，也可以提供 search 方向或者文档链接。
这套纪律在团队里的效果非常直接：评论的争议率下降，review 的通过速度反而变快了。因为作者觉得你是在帮他把代码改得更好，而不是在挑刺。
3.4 基于实际项目的审查流程演示
我用一个简化的示例流程来说明这个过程。假设这是一个权限校验模块的优化改动，核心是引入一个缓存来减少数据库查询。
第一步，作者创建 MR。描述部分这样写：
## 背景 权限校验接口 getPermission 每次请求都会查询数据库，高峰时段 QPS 达到 5000，数据库压力较大。 ## 方案 引入本地缓存（Caffeine），以 userId 为 key，缓存用户权限列表 5 分钟。 考虑权限变更的实时性要求不是极高，5 分钟过期可以接受。 ## 影响面 - 修改文件：PermissionService.java、PermissionCache.java - 关联服务：用户中心、权限中心 ## 测试验证 - 单测覆盖缓存命中与过期场景 - 本地压测对比：数据库查询 QPS 从 1500 提升到 12000第二步，reviewer 开始阅读 diff。按照四层检查法，先看架构层，发现缓存逻辑直接写在了 PermissionService 里，而不是单独抽一个 cache 组件——这个算是建议项，不是阻塞项。再看逻辑层，发现一个问题：缓存过期策略只考虑了 userId，但如果某个用户在同一时刻被修改了权限，旧权限会在缓存中最多存在 5 分钟，这在某些安全敏感的场景里可能无法接受。于是评论提出一个疑问："权限变更入口是否会主动触发缓存失效？如果没有，5 分钟内新权限无法生效。"
第三步，作者看到评论后，回复："目前权限变更接口没有做驱逐逻辑，可以加一个手动失效的接口。"然后补充实现。到这里，这次 review 就完成了最有价值的动作——一个单纯做缓存时没考虑到的权限实时性问题被及时拦截了。
第四步，自动化检查通过、至少一位 reviewer approve、作者按建议处理完所有阻塞项后，合并 MR。
这套流程看起来简单，但每一步都有明确的输入和输出，不像很多团队那样"提交完就等，等到 review 了才发现描述没写清楚"。
4. 常见问题排查与团队落地经验
4.1 为什么总有人 merge 前才疯狂补测试
我排查过很多团队的质量问题，最典型的一个场景是：发布前一天，大家都在紧急补测试用例。仔细聊下来，根源往往不是测试意识差，而是 review 流程里对测试的预期模糊。
我见过一些 PR，代码改了 300 行，描述里写"已自测通过"，但测试目录里一行新代码都没有。reviewer 如果想坚持"没有单测不能合并"，就会造成大量拉锯。但如果放松要求，测试债就会一直堆积。
更好的做法是在团队约定里明确：对于包含新逻辑的 MR，必须至少包含针对核心分支的有效单测或者集成测试；对于纯配置修改、文档修改、依赖升级这类 MR，可以不要求单测，但要写明回归验证方式。把测试要求写清楚之后，大家的执行成本反而降下来了——因为双方都不需要再反复博弈和猜测。
4.2 评论变成"吵架现场"时该怎么收场
代码审查中的一个高发问题：作者认为某个方案是对的，reviewer 也认为自己的方案是对的，两个人在评论区你来我往，谁也说服不了谁。这种时候，如果没有人喊停，局面很容易从技术讨论演变成个人立场之争。
我处理这种情况通常会做三件事。
第一，及时中止评论区的"无限辩论"。技术问题在评论区里讨论超过三轮基本就进入边际效益递减区间了，该从异步转为同步沟通了。拉一个 10 分钟短会，两边直接对话，通常比在评论区打字快得多。
第二，把矛盾聚焦到"具体的用户场景"上。很多争论其实是因为双方脑子里的前提假设不同。比如一个人认为这个接口只被内部系统调用、调用频率很低；另一个人认为这个接口未来可能在公网开放、需要更强的安全保护。这两种假设，用"具体场景"才能把讨论拉回同一条基准线。
第三，如果实在无法达成一致，快速升级决策。让模块负责人或者架构师参与仲裁，而不是让两个开发者在 PR 里耗着。这个仲裁不是"谁官大听谁的"，而是基于清晰的业务优先级和技术取舍记录来做决策，并在 PR 里留下决策理由。
4.3 团队落地代码审查的四个实用门槛
很多团队不是不知道代码审查重要，而是不知道如何开始。这里我把"从零搭建一套审查流程"拆成四步，每一步都不复杂，但缺一不可。
第一步，先做工具层面的基础设施。用 Git 平台自带的 MR/PR 功能就可以了，不需要额外引入复杂的商业审查工具。打开分支保护规则，强制merge_main之前必须经过至少一位 approve，这一步基本上所有主流代码托管平台都支持。
第二步，建立一个简单的审查清单。不用一开始就写一个几十项的大清单，那样没人愿意打开。先定 5 到 8 个关键项，例如"并发安全""事务边界""异常处理""测试覆盖"等，放在模板里，让每一个新 MR 自动带上。
第三步，从"试点模块"开始跑。不要第一天就要求全团队所有项目全部执行新流程，那样阻力会非常大。先挑 1 到 2 个核心模块，让模块 owner 和资深开发先带头跑两周，把流程中的问题暴露出来并修正。
第四步，建立起"review 本身也被 review"的反馈闭环。每季度或者每半年回顾一次：我们的 MR 平均多久被 approve？评论中的阻塞项占比是不是太高？有没有反复出现的 review 评论类型？这些指标能帮你判断，代码审查到底是越来越高效，还是变成了大家都讨厌的绊脚石。
5. 工具与自动化：让代码审查更轻松
5.1 自动检查与代码审查的分工
不少团队对工具的理解有一个误区：觉得上了 SonarQube、CodeQL 之类的自动化代码检查工具，就能替代人工 review 了。实际上这两者的分工完全不同——工具擅长执行"确定性规则"，比如检测明显的代码坏味道、安全漏洞模式、规范违反；而人工 review 擅长处理"上下文相关的设计判断"，比如这个抽象是否合理、那个边界条件是否考虑到实际业务场景。
自动化检查的意义在于"把人从重复劳动里解放出来"。如果一个团队每次 review 都在为"单测覆盖率低于 80%"或者"存在明显未使用的变量"这种问题浪费口水，说明自动化程度不够。这些规则完全可以交给 CI 在 MR 提交时自动检查，并在 MR 中自动留下检查报告。
我见过一个比较成功的实践是：CI 在 MR 打开后自动执行静态扫描、单测、构建，并把结果集中汇总到 MR 页面下方。reviewer 打开 MR 时，第一眼看到的是"CI 红灯还是绿灯"，第二眼再去看 diff。这样做的好处是，reviewer 不用再花时间基本重复的检查，可以把全部精力用在设计层面的思考上。
5.2 自定义脚本：用本地钩子拦截低级问题
除了 CI 里的检查，还可以在开发者本地加一道防线。我常用的是 Git 的 pre-commit 钩子，在代码提交前先做几件事：检查是否误提交了大文件、是否包含了调试日志、是否忘了加单测文件、是否包含密钥等敏感信息。
举一个很实际的小脚本例子。下面是一段用 bash 写的 pre-commit 钩子片段，作用是在提交前搜索暂存区代码中是否有典型的调试残留：
#!/bin/sh echo "Running pre-commit checks..." # 检测调试日志残留在暂存区 if git diff --cached --name-only | grep -E '\.(java|py|js|go)$' | xargs grep -n "printStackTrace\|console\.log\|fmt\.Println" 2>/dev/null; then echo "ERROR: Found debug/logging leftovers. Please remove them before commit." exit 1 fi # 检测是否误提交 .env 或密钥文件 if git diff --cached --name-only | grep -E '\.(env|pem|key)$'; then echo "ERROR: Sensitive file detected in staged files." exit 1 fi exit 0这个钩子拦截不了复杂逻辑问题，但能避免很多非常基础的"低级错误"进入 review 阶段。把这种问题挡在提交之前，双方的心情都会好很多。
5.3 MR 模板与提交信息模板
最后分享一个效果极佳但常被忽略的细节：给团队配置统一的 MR 模板和提交信息模板。Git 平台一般都有能力配置仓库级别的 MR 描述模板，Git 本身也支持commit.template配置。
MR 模板的作用是倒逼作者在创建时就把关键信息补充完整。下面是我们在团队里常用的 MR 模板骨架：
## 背景 （为什么需要这次改动？建议补充关联需求或缺陷链接） ## 改动方案 （总体设计思路，关键实现选择） ## 影响范围 （影响到的模块、接口、数据存储、三方依赖等） ## 测试验证 （单测/集成测试/手动验证步骤） ## 其他说明 （可选：潜在风险、后续 TODO、需要特别关注的点）模板不是限制，是提醒。它最大的价值是让"信息缺漏"变得显眼——如果作者没填"影响范围"，reviewer 就能在第一时间发现并要求补齐，而不是自己在 diff 里摸索半天。
6. 常见问题速查表
我在实际推进代码审查的落地过程中，整理了下面这些问题。遇到类似情况时，可以按图索骥。
| 问题现象 | 可能原因 | 解决方案探索方向 |
|---|---|---|
| MR 长期积压，没人 review | 没指定 reviewer，或者只依赖一个人 | 建立 reviewer 自动指派规则，按模块分配 |
| 大量评论都是格式/风格类 | 缺少自动格式化检查和静态检查 | 引入 formatter 和 lint 工具，把这些交给 CI |
| 作者懒得写描述 | 模板缺失，或者写了也没人看 | 配置仓库 MR 模板，reviewer 遇到缺描述就不 review |
| review 后 bug 还是很多 | 审查只看了 diff，没有理解业务上下文 | 要求作者在描述中补充背景，必要时走查 |
| 评论区吵到无法收场 | 讨论超出异步沟通效率边界 | 换成同步短会，必要时升级决策 |
| 新人不敢评论 | 团队文化被"大佬权威"压住了 | 在回顾会中鼓励提问，强调"没有愚蠢的问题" |
这张表是我自己在实践中最常回看的几类问题。它们不是一次性解决就完事的，团队规模一变、人员一换、项目复杂度一变化，这些问题就会换个姿势重新冒出来。好的一面是，只要流程骨架在，这些问题都有对应的调整方向。
最后再分享一个我个人的习惯：每隔一段时间，我会翻一翻自己最近几个月的 review 历史，统计一下自己提到的评论里，有多少后来被验证为有实际价值的、多少是"当时觉得重要但后来发现无关紧要"的。这个自我复盘动作看起来很简单，但对提升 review 能力非常有帮助。代码审查本质上是一种有方向的阅读，读得多了，自然就能越来越快地看到一个改动背后的设计意图和潜在风险。
