代码审查这个事,说起来每个团队都觉得重要,但真正做起来,能坚持、能做出效果的团队其实不多。我自己在不同规模的团队里折腾过好几轮,从最开始用 GitHub 的 Pull Request 功能,到后来搭过 Gerrit,再到自己写脚本配合 Git 钩子做自动化检查,踩了不少坑,也总结了一些可以复用的经验。今天借“open-code-review”这个主题,把这些年的实践梳理一遍,重点聊聊我们是怎么把代码审查这件事拆开、落地、并且让它真正对工程质量产生作用的。
先说清楚,这篇文章不只是在讲某一个具体的开源工具,而是一套围绕代码审查展开的工程实践思路。无论你现在是用 GitLab、Gitea、GitHub,还是公司内部的代码托管平台,核心方法都是通用的。如果你正准备在团队里推行代码审查,或者觉得现在的审查流程流于形式、没有起到把关作用,这篇文章应该能给你一些可以直接用的方案。
1. 代码审查的整体设计思路:先搞清楚它到底在解决什么问题
很多人对代码审查的理解就是“让另一个人看看代码有没有问题”,这个理解没错,但如果只停留在这一层,审查很容易变成一种走过场。我在实际推动这个事的时候,第一件事不是选工具、定流程,而是把团队对代码审查的预期拉齐。
1.1 代码审查不只是找 bug,更是知识传递和团队共识的载体
代码审查表面上是质量控制手段,实际上它同时承担着至少三件事。
第一,缺陷拦截。这个很好理解,多一双眼睛看代码,逻辑错误、边界条件、资源泄漏这类问题在合入主干之前被发现的概率会大很多。但要注意,审查不应该承担所有质量责任,单元测试、静态检查、持续集成这些自动化手段能拦截的,不应该靠人工去盯。第二,知识传递。新人对系统不熟悉,通过审查老员工的代码,可以快速了解业务逻辑和代码风格;反过来,老员工审查新人的代码,可以及时纠正一些惯性思维。第三,技术共识的落地。代码审查是推动团队编码规范落地最有效的手段之一,比贴文档、开培训会更直接有效。
我在设计整个审查体系的时候,一直在告诉自己:审查是沟通行为,不是审判行为。带着这个前提去设计流程和工具,很多决策就会不太一样。
1.2 审查粒度与节奏:小步快跑优于大版本文书
最容易让代码审查走向形式化的三个典型失败模式,我这些年都经历过。
第一种,大 PR 模式。一个合并请求里面塞了几十甚至上百个文件,投票者看着看着就失去耐心,最后草草点个通过,审查效果接近于零。我见过最极端的一个案例,一次提交包含了数据库迁移、前端重构、后端接口调整三件事,这种审查根本没有办法做。
第二种,集中审查模式。团队习惯在功能开发完、准备发版前集中做一次代码评审,这个模式的问题在于时间跨度太长之后,评审者很难想起当时的上下文,代码和需求之间的关联已经断了,提出的意见往往停留在表面的命名、格式问题上,触及不到真正的设计问题。
第三种,审查者只有一个且固定。单点依赖很危险,这个人请假或者忙于其他事,整个合入流程就阻塞了,更关键的是,固定一个人审查容易形成思维定式,容易漏掉问题。
所以我在推动 open-code-review 这套实践的时候,定的第一个规则就是:小步提交、及时审查。一次合并请求控制在一个逻辑变更单元内,关联的文件尽量少,可以快速完成上下文切换。节奏上鼓励当天提交当天审,最多不超过一个工作日,避免上下文丢失。
2. 工具选型解析:我给团队做了一次重武器和轻量级的对比
工具选对了流程才能落地顺畅,选错了整个团队每天都在抱怨。代码审查工具这块市面上选择其实不少,关键在于匹配团队规模和技术栈。
2.1 主流代码审查平台的能力对比与分析
先说说我用过的几类方案,各自的特点比较明显。
第一类是完整的代码托管平台自带的审查功能,例如 GitHub 的 Pull Request、GitLab 的 Merge Request。这类方案的优势是和代码库、CI/CD 无缝集成,用户体验好,开箱即用;缺点是功能集成度高,但平台锁定效应强,如果你是在内网环境,部署一套 GitLab 需要一定的运维投入。
第二类是独立的审查系统,比如 Gerrit。Gerrit 在开源社区里口碑很好,很多知名项目都在用它。它的核心设计是“不让没有通过审查的提交直接进入代码库”,通过 push 一个 refs/for/branch 的特殊引用,让每一次提交都先经过审查才能合入。这在严格合规的团队里很好用,但学习曲线比较陡峭,命令行操作和 Web 交互对手感比较挑剔,有些习惯了 GitHub 流程的同事会不太适应。
第三类是轻量级脚本化的方案,自己写 Git 钩子、配合 CI 做自动检查。这个阶段我在小团队里试过,优点是灵活,可以按自己的需求定制任何规则,缺点是功能比较原始,缺乏一个统一的界面来回放审查意见、讨论上下文。
我需要说明一下,我做“open-code-review”这个项目的时候,团队规模大概是几十人的技术部门,代码托管用的是 Gitea,因为轻量、自托管方便,预算压力小。Gitea 虽然自带 PR 功能,但团队希望在此基础上增加更细粒度的审查辅助工具。我们调研时重点对比了 Gerrit、Gitea 内置 PR 加外部静态分析工具结合的方式,以及自研脚本配合 CI 的方案。最终我们选择了自研一个轻量工具,这在后文会详细展开。
2.2 为什么我最终选择了自托管加脚本化辅助的路线
先解释清楚我们选型的逻辑。不自建 Gerrit 是因为团队对命令行交互的接受度不高,新员工上手成本高;不直接用 Gitea 内置 PR 是因为我们希望额外的自动化规则能内嵌在评审过程中,而不是事后补检查。
选型时我拟了三个标准。第一,尽量不改动团队现有的提交习惯,保持 git 原生工作流;第二,自动化检查的结果必须回到人工评审界面中,不能是两套割裂的系统;第三,整体方案要能跑在内网环境,代码和数据不出内网。
基于这三点,我们的路线就是:Gitea 作为代码托管和变更讨论的界面,一个定制的 git 钩子配合 CI 流水线,再加上一套脚本化的静态检查与提交信息校验。这套组合已经跑了一年多,效果比较稳定。我把这套实践的核心模板写在后面的章节里,你可以直接复制过去用。
3. 实操过程与核心环节实现:一套可以直接照搬的审查流程
接下来是重头戏,我把自己前前后后搭出来的这套流程完整拆开给你看。整个流程覆盖三个阶段:提交前、审查中、合入后。
3.1 提交前的自动化防线:写清楚提交信息比什么都重要
很多团队不重视提交信息,但提交信息其实是代码审查的第一道门面。审查者打开一个合并请求,第一个看到的就是标题和提交说明,信息写不清楚,审查者根本没耐心看代码。
我们在项目里自定义了一套提交信息格式,并且通过脚本在校验阶段强制约束:
<type>(<scope>): <subject> - 变更动机:为什么做这个改动 - 影响范围:涉及哪些模块/服务 - 测试说明:手动验证了哪些场景、自动化测试覆盖情况举个例子,一个实际提交看起来是这样的:
fix(auth): 修复刷新令牌过期后未跳转登录页的问题 - 变更动机:用户长时间停留在页面后点击操作,令牌过期导致接口返回 401 - 影响范围:auth 模块、前端请求拦截器 - 测试说明:手动验证了令牌过期和刷新两种场景,补充了单元测试覆盖这套格式看起来简单,但执行起来效果极好,因为它强迫提交者在写代码之前就想清楚这三个问题。为了落地这个规范,我写了一个 pre-push 钩子,检查最近一次提交信息是否包含这几个关键字段,不满足的会在推送前拦截并给出提示。
3.2 禁止直接推主分支:一个低级但极其有效的规则
小团队最常见的坏习惯是顺手就往 master 分支推代码,这个习惯不改掉,后面所有的审查流程都无从谈起。我们在 Git 服务端配置了分支保护规则,只允许通过合并请求合入代码到主分支,并且要求至少一位审查者给出明确通过意见。
这里需要看清楚一个关键点:分支保护只是基础设施层面的第一步,真正提高审查质量的是审查者的责任意识,工具只能辅助,不能替代。
3.3 审查脚本的核心实现:用一行行检查指令替代重复劳动
人工审查最花时间的地方在于:格式问题、明显的不规范命名、可疑的写法。这些问题其实可以交给脚本去判断,把人工精力留给真正的逻辑设计和架构问题。
我们写了一个 check-review.sh 脚本,挂在 CI 的流水线里,核心功能包含三个部分。
第一部分是对比分支间的差异文件,找出新增或修改的代码文件,提取出它们所属的模块路径。第二部分是跑静态检查工具,后端用 golangci-lint,前端用 eslint,把警告和错误信息格式化输出,作为评审页面的一个自动评论。第三部分是检查新增代码中是否包含常见的问题模式,比如硬编码密码、调试用的 print 语句、TODO 注释未关联 issue 编号。
脚本的核心逻辑不复杂,一个简化版本的骨架长这样:
#!/bin/bash # 获取当前分支与主干分支的差异文件列表 TARGET_BRANCH="origin/main" CHANGED_FILES=$(git diff --name-only HEAD "$TARGET_BRANCH") # 分别提取前后端代码文件 GO_FILES=$(echo "$CHANGED_FILES" | grep '\.go$' || true) JS_FILES=$(echo "$CHANGED_FILES" | grep '\.\(ts\|tsx\)$' || true) # 执行静态检查,只针对变更文件 if [ -n "$GO_FILES" ]; then echo "Running go vet on changed files..." go vet $GO_FILES fi if [ -n "$JS_FILES" ]; then echo "Running eslint on changed files..." npx eslint $JS_FILES fi # 检查是否包含明显有问题的代码模式 echo "$CHANGED_FILES" | while read -r file; do if [ -f "$file" ]; then grep -nE '(password|secret|api_key)\s*[:=]\s*["\x27]' "$file" && echo "发现疑似硬编码密钥:$file" fi done echo "Review pre-check completed"实际使用中脚本远比这个复杂,我们还会结合 Fish 和容器的版本锁定,在干净的容器环境里跑上述检查,避免本地环境差异导致结果不一致。
提示:脚本的实际能力不在于写得多炫酷,而在于把团队最常犯的问题提炼成可执行的规则。每季度新增五条以内的规则,比一次性堆砌大量规则效果更好,因为团队成员能真正记住并遵守。
4. 审查环节的工作流设计:从指派到合入的完整闭环
合入前的一切自动化都只是铺垫,真正的核心环节还是人与人之间的讨论。这一节我把审查工作流的具体设计完整说清楚。
4.1 审查者分配逻辑:谁来审,审哪些内容
审查者分配我总结了三个原则:模块负责人优先、利益相关方参与、路由和配置分享。
模块负责人优先,就是说改到哪个模块的代码,优先指派维护这个模块的人来审查。这要求团队内部需要维护一个模块归属清单。利益相关方参与,是说如果这次变更涉及数据库表结构、API 接口协议、前端页面交互,那前后端负责对接的人也应该被加入审查人列表。第三个原则是路由和配置分享,主要是防止出现单点阻塞,确保审查者至少有两个人,任何一个人请假都有其他人可以顶上。
审查内容的划分比分配原则更重要。我通常把审查视角分成几个层次:设计正确性、逻辑完整性、安全性和效率、可维护性。其中设计正确性是最重要也最难的,它关注的是这次改动和系统整体架构是否一致,有没有偏离原有的技术方向。
4.2 审查意见的表达技巧:客观描述,不要主观评判
老生常谈,但真的很影响协作体验。我见过很多团队因为审查意见的措辞问题导致团队内部关系紧张。
这里有一个我自己的经验:审查意见尽量用“当前实现存在什么问题 + 为什么这是一个问题 + 可以怎么改”的三段式表达,而不是简单地说“写得有问题”。
比如,“这里如果用户输入为空,会触发空指针异常,建议在方法入口加一个非空判断”就比“这个代码有 bug”要好得多。后者既没有说明问题场景,也没有给出解决方向,对方收到这种反馈只会觉得被否定。
我们在 open-code-review 的流程里加入了一个引导模板,开发者提交审查时,页面上会显示建议填写的说明结构;审查者留下建议时,也会被提醒使用描述性语言而不是评论式语言。这个不依赖强制检查,持续引导比强行规定更能形成团队文化。
4.3 合入规则与兜底策略:一个合入清单,降低遗漏风险
为了让审查过程有据可依,我把合入条件做成一个 checklist,开在每一个合并请求的描述里。
- 变更描述信息完整,包含动机、影响范围和测试说明
- 关联的自动化检查全部通过,没有引入新的告警
- 至少一位模块负责人已审批通过
- 与主分支的最新代码没有冲突
- 依赖的接口变更已经通知到对应的消费者
这份清单看着简单,但在实际中帮我拦住过不少问题。有些合并请求的代码本身写得不错,但提交说明里没写测试内容,一查果然没跑测试。有了清单,相当于给了审查者一个最低限度的行为底线,审查者逐项核对,不会因为太信任对方而漏掉关键检查。
5. 常见问题与排查技巧实录:踩过的坑都在这里了
这一部分的内容全部来自实际生产环境里的问题,不是理论推演。
5.1 代码审查被形式化,大家都不认真看怎么办
这是推行审查制度后最常遇到的挫败。认真写代码的同事发现自己的 PR 被快速合并了,审查者可能并没有仔细看,刚开始大家还有热情,时间长了就变成走流程。
我的做法是把审查质量纳入团队的月度复盘和绩效盘点,这不是为了考核而考核,而是为了奖励那些真正发现了有价值问题的审查者。有一次我们推动团队对几个历史 bug 做回溯,发现有两个问题其实是可以在审查阶段被拦截的,只是当时审查者因为时间紧张顺手点了通过。我们把这个问题拿到例会上讨论,提醒大家:审查是为了帮你提前发现线上问题,不只是走完一个流程。
在技术手段上,我会在 CI 里统计审查者的评论数和代码入库前的修改率,这些数据汇总到月度报告里,帮助团队观察审查的活跃度变化趋势,如果某个人的评论数长期为零,会私下沟通了解一下原因,是觉得代码没问题顾不上,还是觉得提意见会被抵触。
5.2 大合并请求拆不动:长功能分支的常见解法
功能分支开了一两个月,累计的改动量非常大,这个时候拆 PR 已经晚了。能做的补救有两类。
一类是先把不影响主流程的改动单独摘出来合并。比如重构了一些公共方法、调整了目录结构、补了测试用例,这些可以拆成独立的合并请求先行合入。另一类是采用增量审查的方式,不要求一次把这个大 PR 全部看完才能合入,而是允许它先合入到一个集成分支,然后在这个分支上分模块进行审查和修正。
这个方案需要团队对合并有一定的信任度,建议配合第 3 节的自动化检查来把关,确保合并进来的代码基础质量是达标的。
5.3 自托管审查工具的部署与维护避坑指南
如果你也计划在自托管平台上搭一套自己的审查辅助系统,以下几个问题值得提前留意。
服务器资源方面,Gitea 本身非常轻量,但如果你在上面跑 CI 和静态检查,就需要额外准备任务执行器。我们一开始用了一台 2C4G 的小机器,跑了几个月后发现 CI 任务排队严重,升级到 4C8G 之后流畅多了。如果团队规模不大,建议使用共享的云主机来跑 CI,不要和代码托管放同一台机器,便于隔离和扩容。
数据备份方面,代码数据是开发团队的命根子。Git 仓库本身的容错性还不错,但审查记录、PR 描述、评论这些元数据存在数据库里,备份必须覆盖到。我们用了每小时的数据库增量备份加每天的完整备份,发生过一次数据库损坏的意外,靠着备份恢复了。备份好不好用,要定期做恢复演练,别等到需要恢复的时候才发现备份文件是坏的。
5.4 常见问题速查表
下面这个表格在其他团队做推广时也发过,可以帮助落地时快速排查问题。
| 问题现象 | 可能原因 | 排查思路与解决方式 |
|---|---|---|
| 合并请求一直不通过 CI | 静态检查脚本依赖的环境变量为配置 | 在 CI 任务里补充环境变量,并在本地先执行一遍脚本确认通过 |
| 提交信息规范校验失败 | 提交信息格式不匹配 | 查看校验规则提示(脚本会打印符合要求的示例),用 git commit --amend 调整后重新推送 |
| 审查者不活跃 | 缺少明确的审查指派规则 | 运维侧优化分配逻辑,规则上要求模块负责人必须参与审查且列表内至少两人可处理 |
| 审查意见偏离代码本身 | 缺乏沟通准则引导 | 在团队约定的规范文档中增加审查意见表达示例,代码审查系统里补充引导提示 |
| 自动检查工具误报过多 | 规则配置过于严格或自定义项不兼容 | 调整规则阈值,对涉及误报的场景在配置中加入排除项,并在团队内公开记录误报情况逐步收敛 |
6. 审查后的总结与改进:把经验工具化为下一个迭代的输入
代码审查不应该在合并后戛然而止。我的习惯是每两周抽了半天时间,和团队一起把最近一段时间审查记录里出现频率比较高的问题做一次回顾和分类。
如果发现最近多个合并请求都存在同一类问题,说明这个类型的问题靠单个人的审查和自觉解决不了,我会把它转化成一条自动检查规则或者一个编码规范条目,沉淀下来。比如我们团队曾有段时间频繁出现测试用例遗漏的覆盖场景,后来在审查清单和 CI 脚本里都加了“涉及分支逻辑的改动必须补充对应用例”的提示,情况很快得到改善。
这个动作的本质是让代码审查产生复利。人解决的问题会被沉淀成自动化规则的输入,自动化规则减少人在基础检查上的时间消耗,从而有更多精力去做真正意义上的架构评审。这是一个正向循环,开始推动时需要一些耐心,但坚持下去,团队整体对“代码质量”的感知会越来越具体,不再是一句口号。
最后再分享一个我实操中很有用的习惯:每过一段时间,我会把合并到主分支的提交历史翻出来,随机挑几个提交看它的审查讨论记录。这个过程有点像考古,能看出来哪段时间团队状态好、哪段时间大家都比较疲惫,审查意见的质量明显下降。一旦发现这个趋势,我就会在团队里提醒大家调整节奏,给审查留出足够的时间。代码审查这件事,说到底靠的是流程加工具加人三者的配合,工具解决流程问题,但最终决定它好不好的,还是人心和习惯。