1. 先说说我为什么要折腾这套 open-code-review 流程
代码审查这件事,几乎每个技术团队都会挂在嘴边,但真正做好的没几个。我做了十年研发,也带过几年团队,见过太多“review 走形式”的案例:PR 挂了两天没人理,reviewer 随手点个 approve,或者反过来,一群人为了代码风格吵到凌晨。问题不在人,在于流程没有把“机器该干的活”和“人该干的活”分开。
所以我自己动手搭了一套开源的代码审查流程,取名就叫open-code-review。它的定位很简单:不依赖任何商业工具,完全基于 Git 工作流、开源 CLI 工具和仓库内模板约定,把团队内部的 code review 从“随缘模式”变成“有章可循”。这套流程在我自己维护的几个项目和几个朋友的小团队里实际跑了大半年,效果比预想中好很多,所以整理出来分享给有同样困扰的人。
这篇内容适合谁看?三类人:一是中小团队的技术负责人,正在头疼 Code Review 流程落不了地;二是开源项目维护者,想让贡献者少犯低级错误;三是刚接触团队协作开发的程序员,想知道一个“正经”的代码审查流程到底长什么样。我会把整体设计思路、机器检查配置、人工审查规范、完整落地步骤和踩坑记录都写清楚,你照着抄就能用。
2. 整体设计:机器先行,人情味留在人工环节
2.1 分层的审查链路是怎么设计的
先说设计理念。传统 review 最大的问题是把所有责任堆在 reviewer 一个人身上,让他在一次审查里既看格式、又看逻辑、还要猜业务背景。这根本不合理,因为人的注意力是有限的,你在低价值问题上消耗了精力,高价值的问题就会被忽略。
open-code-review 的核心分层逻辑是:
- 第 0 层:本地准备。开发者在提交代码之前,由 Git 钩子自动完成提交信息规范校验、代码格式化、基础静态检查。
- 第 1 层:CI 自动检查。PR 创建后,流水线自动跑完整静态检查、单元测试、依赖安全检查,把明显的问题全部挡在外面。
- 第 2 层:人工审查。PR 通过所有自动检查后,reviewer 只需要聚焦业务逻辑、架构设计、数据一致性、安全隐患这些真正的“人类判断”问题。
这个分层最大的好处,是把 reviewer 的精力从“找毛病”转变为“看设计”。曾经有团队成员跟我说,以前 review 一半时间在纠结缩进和命名,现在这些事机器全干了,他只需要认真理解代码逻辑,一次 review 的时间反而缩短了,但质量明显提升。
2.2 为什么坚持用开源工具链
最开始我也犹豫过,要不要直接用市面上的商业代码托管平台自带功能,或者买一套企业级审查工具。后来对比了一圈,发现几个现实问题:商业工具是按席位收费的,团队稍微大一点成本就不低;平台的审查规则是黑盒,出了问题只能提工单;最关键的是,商业工具很难跟团队现有的开发工作流深度绑定。
所以我的选择是:全部用开源方案。Git 本身是开源的,husky 负责 Git 钩子,commitlint 管提交信息,ESLint 和 Prettier 管代码规范,GitHub Actions 做 CI。每个环节都是业界用得最多、资料最全的工具,出了任何问题都能自己排查。这套方案也完全不挑托管平台,GitHub、GitLab、Gitea 都能跑,以后团队换平台,流程文件直接带走,没有任何迁移成本。
2.3 每个角色在流程里的分工
任何流程设计到最后,本质都是定义“谁在什么时候干什么事”。open-code-review 里角色分成三类:
- 作者(PR 创建者):负责写清楚提交信息、按模板填写 PR 描述、保证本地检查通过。
- 审查者(Reviewer):负责认真阅读 diff,按审查清单逐项确认,给出明确结论。
- 维护者(Maintainer):负责合并 PR,确保讨论闭环、CI 通过、没有“挂起”的 review 意见。
每个角色都有明确的交付物,而不是一句空泛的“大家自觉 code review”。这个表面上看是流程正规化,实际上是在保护每个角色的时间和精力。
3. 自动化层落地:把能交给机器的都交给机器
3.1 约定式提交与提交信息检查
代码审查最常见的低质量输入就是“鬼都看不懂的提交信息”。一条fixed bug或update,reviewer 根本不知道这次改动是为了什么。所以在 open-code-review 里,第一道关卡就是提交信息规范。
我用的是约定式提交规范,格式很统一:
type(scope): subjecttype 必须是feat、fix、docs、style、refactor、perf、test、chore这些限定词,scope 是可选的模块名,subject 是一句话描述。配合 commitlint 做强制检查,配一个commitlint.config.js文件搞定:
module.exports = { extends: ['@commitlint/config-conventional'], rules: { 'type-enum': [2, 'always', [ 'feat', 'fix', 'docs', 'style', 'refactor', 'perf', 'test', 'chore' ]], 'subject-min-length': [2, 'always', 10], 'subject-max-length': [2, 'always', 100] } };注意我加了两条自定义规则:subject 最短 10 个字符、最长 100 个字符。最短限制是为了逼开发者把提交描述写清楚,最长限制是为了防止有人在提交信息里写长篇论文。
提示:提交信息写得好不好,直接决定你后续
git bisect、版本回滚、生成 changelog 时的体验。我见过太多项目因为提交信息一塌糊涂,半年后回头查问题根本无从下手。
3.2 静态检查与格式化配置
静态检查选型上,我以 ESLint 为主,Prettier 负责格式化。两者各有分工:ESLint 管的是“代码哪里有问题”,Prettier 管的是“代码长什么样”。ESLint 的规则配置不建议全量开启所有规则,那会把人逼疯。我实际用的是一套偏保守但有效的配置:
{ "parserOptions": { "ecmaVersion": 2022, "sourceType": "module" }, "extends": ["eslint:recommended"], "rules": { "no-unused-vars": "error", "no-undef": "error", "eqeqeq": ["error", "always"], "camelcase": ["error", {"properties": "always"}], "complexity": ["warn", 10] } }这里有两个值得解释的选择。eqeqeq强制严格相等,是防隐式类型转换的经典坑;complexity限制圈复杂度到 10,超过就给警告,这是倒逼开发者写小函数。如果函数写得太长、分支太多,我宁可在 CI 里让它先跑红,让开发者自己拆代码,而不是把一堆复杂逻辑塞进 PR 里让 reviewer 硬着头皮看。
Prettier 这边没什么好争论的,直接开默认配置就行。团队里最容易引发争吵的就是代码风格,既然 Prettier 能自动统一,就别在这上面浪费人力。我唯一改动的配置是单行长度,从默认的 80 改成 100,因为现代开发显示器普遍宽屏,80 字符会让代码换行太频繁,反而不利于阅读。
{ "semi": true, "singleQuote": true, "printWidth": 100, "trailingComma": "es5" }3.3 CI 工作流里一键跑完整套检查
本地钩子能拦住一部分问题,但不能全指望它,因为总有开发者会跳过钩子,或者在本地环境有各种奇怪的问题。所以必须在 CI 里再跑一遍,作为最终的强制红线。
我用的 GitHub Actions 工作流大概长这样:
name: code-review-checks on: pull_request: branches: [ main, develop ] push: branches: [ main ] jobs: quality-checks: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - name: Setup Node.js uses: actions/setup-node@v4 with: node-version: 20 cache: 'npm' - name: Install dependencies run: npm ci - name: Lint run: npm run lint - name: Check formatting run: npm run format:check - name: Test run: npm test -- --coverage - name: Commit lint on PR title run: npx commitlint --from=HEAD~1 --to=HEAD这个 workflow 的好处是配置清晰、可扩展。以后要加依赖安全检查、镜像扫描、类型检查,都是往 steps 里加一项的事情。而且你注意到一个细节:我在 PR 触发和 push 到主干时都会跑,这保证了主干上永远只有通过检查的代码。
3.4 自动化层要达到什么标准才算合格
很多团队搭自动化检查,搭完之后只是“有个东西在那”,并没有真正发挥拦截作用。我总结了一个合格标准:如果一个 PR 出现了格式错误、未使用变量、测试失败这三种中的任何一种,它就不应该出现在 reviewer 的待办列表里。
这个标准听起来很基础,但真能做到的团队不多。常见的情况是 CI 处于“结果仅供参考”状态,或者合并按钮对所有人开放,导致自动检查形同虚设。在 open-code-review 里,我会把合并权限收拢到维护者手里,并且在仓库设置里开启“分支保护规则”,要求 PR 必须通过所有 CI 检查才能合并。这一道闸门,比你在流程文档里写一百遍“请保证代码质量”都管用。
4. 人工审查层:真正让代码质量提升的地方
4.1 Pull Request 描述模板怎么设计
人工审查要高效,第一件事是让 PR 自解释。如果 reviewer 还要自己浏览一遍代码猜测“这个 PR 想干嘛”,那 review 效率直接打五折。我在仓库里放了一个.github/PULL_REQUEST_TEMPLATE.md,强迫每个 PR 写清楚三件事:改了什么、为什么改、怎么测试的。
模板内容大致长这样:
## 变更描述 (这段改了哪些功能,解决了什么问题,不要贴代码,用自然语言描述) ## 关联需求/Issue (写 issue 编号或对应需求链接) ## 测试验证 - [ ] 本地单元测试通过 - [ ] 手动验证了关键场景 - [ ] 对现有功能无影响 ## 影响范围与风险 (哪些模块可能受影响?有没有破坏性变更?) ## 截图或日志 (UI 变更必须贴截图;接口变更必须贴请求/响应样例)这个模板最核心的是“影响范围与风险”这一栏。很多开发者在写 PR 时只写“我做了什么”,不写“改动可能影响什么”,结果 reviewer 得自己在代码里找雷。加上这一栏之后,reviewer 的效率明显提升,因为他知道重点往哪儿看了。
4.2 给 reviewer 的检查清单
人工审查不能靠感觉,必须有一份可以打勾的检查清单。我总结了一份Code Review Checklist,写在仓库的docs/review-checklist.md里,reviewer 每次审查前过一遍:
- 功能逻辑:这段代码是否实现了 PR 描述中的目标?有没有漏掉边界条件?
- 可读性:变量名和函数名是否自解释?有没有需要注释才看得懂的魔法数字?
- 异常处理:网络请求、文件读写、外部服务调用有没有错误处理?是否会导致静默失败?
- 性能隐患:有没有 N+1 查询?有没有在循环里做高开销操作?
- 安全性:用户输入是否被直接拼进 SQL、HTML、命令行?有没有权限校验遗漏?
- 测试覆盖:新的逻辑有没有对应的单测?异常路径是否被测试覆盖?
真正开始执行时,我会建议 reviewer 先回答最后一个问题:如果这段代码半年后出问题,最大的隐患可能在哪里?这个问题会强迫 reviewer 从维护视角去理解改动,而不是仅仅停留在“这段代码能不能跑”。
4.3 审查优先级:先看什么后看什么
人的注意力是有限的,我建议 reviewer 按这个优先级来分配精力:
第一优先级是安全和数据相关逻辑。资金、个人信息、权限校验这些代码出错,后果是灾难性的。这部分要逐行看,不能跳着看。
第二优先级是核心业务逻辑。比如订单状态流转、用户状态切换这类,一旦逻辑错误,影响面很大。
第三优先级是异常处理和边界条件。多数 bug 不是发生在正常路径,而是发生在用户输入极端值、外部服务超时、文件不存在这些非常规场景。
第四优先级才是代码风格、变量命名、函数拆分。这些其实机器已经检查过一大部分了,reviewer 只需要关注那些需要判断力的部分,比如命名是否准确反映业务含义。
按照这个顺序,即使 PR 特别大,reviewer 也能保证在最重要的部分投入足够的注意力。
4.4 如何让 review 意见更少火药味
这条虽然不在“代码”的范畴,但我觉得必须写。因为代码审查本质是人与人的沟通,方法不对,再好的流程都会变成形式主义。
我的经验是:review 意见尽量“对事不对人”,少用命令式语气,多用提问式语气。比如把“这里写错了”改成“这里是不是有个边界情况没考虑到?”;把“这个变量名太差了”改成“这个变量名我读起来有点困惑,是不是能体现出它的业务含义?”。语气从“挑刺”变成“讨论”,对方接受度高很多。
另外,每个 review 意见一定要写清“为什么”。光说“这段代码不好”等于没说,你得指出问题、给出理由、最好再给一个改进建议。我在团队里定的规矩是:每个 review comment 至少要包含问题定位和理由这两部分,缺少任何一部分的意见不输出。
5. 落地实操:从零搭一套 open-code-review 的全过程
5.1 初始化项目与目录结构
现在进入真正可操作的部分。这套流程不要求你从零开始搭框架,我用一个最简单的 Node.js 项目演示完整配置,你对应到自己的技术栈就行。先初始化项目:
mkdir open-code-review-demo cd open-code-review-demo npm init -y git init然后安装所有需要的开发依赖:
npm install -D eslint prettier husky lint-staged @commitlint/cli @commitlint/config-conventional注意这里我额外装了一个 lint-staged。它的作用是在提交时只检查本次暂存区里改动的文件,而不是全项目扫描。大型项目全量 lint 可能要好几分钟,lint-staged 能把耗时压到几秒,体验差异非常大。
推荐的项目结构是这样,杂七杂八的东西都放对应目录:
open-code-review-demo/ ├── .github/ │ ├── PULL_REQUEST_TEMPLATE.md │ └── workflows/ │ └── code-review-checks.yml ├── docs/ │ └── review-checklist.md ├── src/ │ └── index.js ├── .eslintrc.json ├── .prettierrc.json ├── commitlint.config.js ├── package.json └── README.md5.2 配置 husky 与 commitlint
husky 新版用了不同的初始化方式,不再写.huskyrc,而是通过命令直接生成钩子文件:
npx husky init生成.husky目录后,给 pre-commit 钩子配上 lint-staged,给 commit-msg 钩子配上 commitlint:
npx husky add .husky/pre-commit "npx lint-staged" npx husky add .husky/commit-msg "npx --no -- commitlint --edit \$1"这里解释一下为什么用npx --no:为了避免 husky 钩子里递归调用自身的 npx 命令导致问题。这是我在实际配置时踩过一次的细节,官方文档没特意强调,你不注意的话在部分 npm 版本上会报奇怪的错。
package.json 里的 lint-staged 配置比较直接:
{ "scripts": { "lint": "eslint .", "format": "prettier --write .", "format:check": "prettier --check ." }, "lint-staged": { "*.{js,ts,vue}": ["eslint --fix", "prettier --write"] } }5.3 配置 ESLint 与 Prettier
ESLint 配置在.eslintrc.json里,我建议直接基于上一节那份改。现在前端生态普遍用 flat config,但对于 Node 老项目或初学者,.eslintrc.json的兼容性更好,团队上手成本低。
Prettier 配置在.prettierrc.json:
{ "semi": true, "singleQuote": true, "printWidth": 100, "tabWidth": 2, "trailingComma": "es5" }配置好之后建议做一次全量格式化,把存量代码先统一风格,之后新代码就不会再混入风格差异:
npx prettier --write .这一步可能会产生一个巨大的 diff,但值得。它一次性把整个代码库的风格基线拉齐了,让后续的增量检查变得干净。
5.4 配置仓库模板文件
接下来创建.github/PULL_REQUEST_TEMPLATE.md和docs/review-checklist.md,内容直接参考第 4 节给出的模板。这里有三个细节值得注意:
第一,模板不要写得太长。我见过有些团队的 PR 模板有十几栏,开发者填到最后直接放弃治疗,复制粘贴一堆无意义内容。控制在五到六个核心栏目就够。
第二,模板里的 checkbox 要少用。每个新增的 checkbox 都是在增加作者的心理负担。只有当一项内容确实重要到必须强制检查时,才应该出现在模板里。
第三,Checklist 文档要放在 reviewer 容易看到的地方,最好是 PR 模板里加一个“请按 Checklist 审查:docs/review-checklist.md”的链接,而不是让每个人自己去找。
5.5 接入 CI 流水线
最后把 CI 工作流扔到.github/workflows/code-review-checks.yml。这里有个细节值得讲:CI 和本地钩子的检查要存在差异。本地钩子跑 lint-staged,只检查暂存区;CI 跑全量检查和测试,保证代码库整体健康。两者配合而不是重复。
工作流配置好之后,去仓库页面开启分支保护规则(Settings → Branches → Add rule),勾选:
- Require a pull request before merging
- Require status checks to pass before merging
- Require conversation resolution
这三项组合起来的效果是:任何 PR 必须在 CI 全绿、没有被“挂起”的 review 意见的情况下才能被合并。有了这个硬性约束,整个流程才真正闭环。
6. 踩坑实录与提速小技巧
6.1 我实际踩过的四个坑
这套流程我已经在不同项目里跑了大半年,踩过的坑写出来,希望你能跳过。
第一个坑是husky 钩子不生效。很多新手在 Windows 上装完 husky 之后发现钩子根本不跑,原因是新版 husky 依赖.git/hooks里的软链接,Windows 上部分环境(尤其是 Git Bash 和权限受限的终端)没法正确创建软链接。解决办法是检查core.hooksPath:确保结果是.husky/_而不是空值。如果不对,重新跑一次npx husky install。
第二个坑是commitlint 对 PR title 无效。本地 commitlint 管的是 commit message,但很多团队实际上用 PR title 代替 commit message(比如用 squash merge)。如果你的仓库是这种合并策略,就得在 CI 里额外对 PR title 做一次检查。GitHub 官方没有现成的 action,我用的方案是让工作流拿到 PR title,再用 commitlint 逻辑跑一遍。
第三个坑是lint-staged 和 ESLint 的缓存冲突。ESLint 默认不缓存,但 lint-staged 在某些 npm 版本组合下会漏掉部分文件。解决办法是不要用 ESLint 的--cache参数,保持 lint-staged 每次都完整扫描暂存区的文件。虽然牺牲了一点增量速度,但换来的是不漏检。
第四个坑是全员试图绕过流程。这不是技术问题,而是管理问题。总有人觉得“我就改了个一行代码,也要走全套流程?”,于是提交时用git commit --no-verify跳过钩子,或者 PR 合并时单选“绕过检查”。这种事只要发生一次,其他人马上会学。我的处理方式是两条腿走路:技术上,分支保护规则里把 root 的权限单独收紧,不让绕过成为可能;管理上,跟团队成员讲清楚流程的价值,而不是把人当机器。
6.2 常见问题速查表
| 现象 | 可能原因 | 排查方法 |
|---|---|---|
| 钩子不执行 | husky 安装失败或 core.hooksPath 不对 | 运行git config core.hooksPath确认是.husky/_ |
| commitlint 不拦截 | commit-msg 钩子未添加或路径错误 | 查看.husky/commit-msg文件是否存在且内容正确 |
| CI 里 ESLint 通过但本地报错 | 依赖版本不一致 | 统一使用package-lock.json或npm ci替代npm install |
| Prettier 和团队已有风格冲突 | 存量代码风格混乱 | 首次配置时先全量格式化,再启用 CI 检查 |
| PR title 不符合规范 | commitlint 只检查 commit | 在 CI 中添加 PR title 检查步骤 |
| 合并按钮显示不可用 | 分支保护规则未满足 | 检查 CI 状态、review 意见、conversation 是否全部闭环 |
这张表基本覆盖了我收到的日常问题。遇到奇葩情况,第一反应是看日志,第二反应是本地复现,不要上来就改配置。
6.3 让代码审查真正跑起来的建议
流程搭好只是第一步,真正让团队养成自觉是更长期的事。我的建议是:先在小团队或者一个不紧急的项目上试点,跑一两个迭代周期,把流程里所有不舒服的地方调顺了,再推广到全团队。千万不要瞬间铺开,不然成员抵触情绪会非常大。
团队刚引入这套流程时,我给自己的定位是“守门员”:前两周我不催任何人,但只要有人提交不合规的东西,我第一时间私聊指出问题,而不是在群里公开批评。两周之后,大家习惯了新规则,流程就开始自行运转。三个月后,团队里新来的成员都是老成员手把手带出来的,根本不需要我再盯着。
最后再分享一个小技巧:代码审查的标准不是“代码写得完美”,而是“下一个维护者能看懂并安全修改”。你把审查标准定成前者,团队会被逼疯;定成后者,大家反而会主动追求更清晰的代码。这算是这个项目里我最大的心得体会。