上个月我重新统计了一次团队的 review 数据发现一个挺扎心的事实PR 平均要 26 小时才有第一个评论其中 40% 的 PR 是被作者 三遍以上才有人看而最终 approve 里有一大半只是LGTM两个字。这个局面不是某一个人的问题是整个团队缺少一套把代码审查从靠人品变成靠流程的机制。我后来花了三周时间搭了一套名为 open-code-review 的开源实践框架把角色、门禁、自动化检查、检查单全部固化到仓库里。这篇文章就围绕这套东西聊聊我到底怎么设计、落地过程中踩了哪些坑以及如果要复刻这套方案最该注意的几个关键决策。1. 评审走过场的团队问题到底出在哪1.1 LGTM 刷屏只是表象根子在三个地方先说症状。我见过不少团队嘴上喊着我们要认真做 code review实际上一周后 review 就变成了流水线操作打开 PR随便翻两眼留下一句LGTM点 approve。这能怪 reviewer 不认真吗我觉得不能。根子在三个地方第一没有门禁approve 和不 approve 没有区别。既然不管怎么点代码最后都会合并那谁还愿意为一个没有后果的动作花时间第二reviewer 不知道自己的责任边界。没人告诉他这次 review 应该重点看什么、看到什么程度算合格他自然选择最省力的方式——跟着感觉走。第三没有人对 review 的时效性负责。PR 挂三天没人理作者催一下大家碍于面子才动手与其说是流程不如说是人际关系。这三个问题叠加在一起最终呈现出来的就是 review 流于形式。但我要强调一句这不是人懒是结构问题。你想靠提高觉悟解决它基本没戏得用工程手段。1.2 review 是一项有时间预算的工程活动很多人把 code review 理解成看完代码之后给个评价这个理解太窄了。在我看来review 是一项正经的工程活动它有投入、有产出、有质量要求也有时间预算。既然工程上有测试覆盖率这种指标那 review 也应该有自己的度量标准比如首次响应时间、平均审查时长、一个 PR 需要几轮 review 才能合并、approve 里有多少是带实质评论的。光有这个意识还不够。你得把标准拆到足够细细到工具能自动统计细到每个人打开 PR 的时候知道自己该干什么。这就是我做 open-code-review 的出发点不是发明一套新的审查理论而是把已经被验证有效的做法模板化、自动化让每个仓库都能低成本用起来。1.3 大多数团队缺的不是平台是配置和规范可能有人会说我们用了 GitLab MR 或 GitHub PR 的审查功能为什么还是乱因为平台提供的是能力不是行为准则。PR 可以设置 required reviewers但如果你不定义谁该出现在 reviewer 列表里这个配置就是空的代码可以写 comment但如果你不规定 unresolved thread 不能合并评论就只是评论。所以 open-code-review 的定位很明确它是一套配置 规范 自动化的组合包包含工作流定义、审查门禁、静态检查脚本、Danger 规则、检查单模板、角色说明文档。你拿过去改改仓库名配好 token跑起来就能用。它不会替你做决定但会确保该做的动作被强制执行。2. open-code-review 的流程骨架角色、阶段与门禁怎么设计2.1 把一次 PR 的生命周期切成六个明确状态我设计流程的第一步是把 PR 从创建到合并的人生给捋顺。没有明确状态机人就只能在挂起和合并之间随机摇摆。open-code-review 里给 PR 定义了六个状态Draft还在写代码、调 CI不具备审查条件不通知 reviewer。In Review标记 ready 之后进入审查队列此时自动评论通知 reviewer。Approved满足 approve 数和全部检查等待合并。Merged代码合入主干。Stale超过 48 小时没有 reviewer 响应自动升级提醒。Blocked存在 unresolved thread 或多位 reviewer 明确提出 Block不允许合并。状态机的好处是任何人在任何时刻都能回答这个 PR 卡在哪一步。团队开会不再靠猜直接刷一眼状态列表就行。实现上GitHub 环境我用 GitHub Actions 监听pull_request事件和pull_request_review事件来切换状态。核心逻辑不复杂关键在于你要坚持状态决定下一步动作的原则而不是让状态只当个展示牌。2.2 三种角色的责任划分谁也不能甩锅流程能不能跑起来角色定义比什么都重要。open-code-review 只保留三种角色Author、Reviewer、Maintainer。Author 的任务不是提交代码然后消失而是要保证自己交出去的东西可被高效审查。具体来说PR 描述里必须写清楚背景、改动范围、测试方式和可能的副作用。提交粒度要小一个 PR 解决一件事改动行数建议控制在 300 行以内。自己先跑两遍 diff明显的问题自己在本地解决掉不要把 reviewer 时间浪费在低级错误上。Reviewer 的任务是对变更的正确性和安全性负责。注意这里不是帮作者改代码而是判断这个变更是否达到合并标准。所以我要求 reviewer 把评论分成两类必须阻塞Block和建议调整Nit/Suggestion。前者数量极少只留给正确性、安全性和架构层面的问题后者只是锦上添花作者可以自行决定是否处理。Maintainer 的职责是处理争议和合并。当两个 reviewer 意见冲突或者 reviewer 与 author 对某个做法争执不下时Maintainer 要做技术仲裁不能放任 thread 无限长出千字长文。2.3 门禁规则允许合并的前提条件open-code-review 默认给 PR 合并设置了四道门至少 2 个 Approve核心库要求 3 个且 Approver 不能是 PR 作者本人。所有 CI 检查必须通过包括静态检查、测试、构建。不允许存在未解决的讨论线程。PR 改动超过 500 行时必须手动说明为什么不能拆小。这四道门在 GitHub 里可以直接用 Branch protection rules 实现在 GitLab 里用 Merge request approvals 配置。我额外用了一个小脚本对500 行这条做了软校验超过行数就自动打一个 warning 标签但不强行阻断——因为确实偶尔有无法避免的大改动。提示门禁的价值不在于卡人而在于让所有人明确一个共识——能够合并的代码不是哪个人拍板的而是达到了一组事先约定好的标准。2.4 超时升级机制让 PR 不被无限搁置很多团队的 review 阻塞根源不是意见一致与否而是压根没人响应。我在 open-code-review 里加了一个超时升级机制PR 进入 In Review 状态后如果 24 小时没有第一个评论机器人自动在群里提醒一次如果 48 小时没有任何动静自动升级给 Maintainer由他调度资源或者直接转移给其他 reviewer。这个机制看起来简单实际效果非常大。它把reviewer 消极怠工从个人品德问题变成了流程自动处理的事件大家反而没什么心理负担。因为提醒的是机器人不是你 某个人面子问题被绕开了。3. 自动化审查链路静态检查、CI 门禁与机器人提醒3.1 给自动化分五个层级别一口气全上做自动化的第一原则不要试图让工具替你当 reviewer。工具能抓住的问题是有限的、可列出来的人的时间应该花在工具表达不了的地方。我在 open-code-review 里把自动化链路分成五层从低到高层级工具作用L1pre-commit / husky提交前把明显问题挡住L2静态检查ESLint/Checkstyle/GolangCI-Lint检查语法、风格、潜在 bugL3密钥扫描gitleaks/TruffleHog防止敏感信息泄露进仓库L4Danger / CodeRabbit 机器人对 diff 做模式化评论L5人工审查处理自动化无法判断的问题大多数团队只看 L2 和 L5中间三层要么靠运气要么靠人肉翻。open-code-review 做的事情就是把 L1 到 L4 全部配好让 L5 的人类把精力集中在这个代码的抽象边界对不对这个数据库操作在并发场景下会不会炸这类真正需要判断力的问题上。3.2 增量静态检查只跑 diff别跑全仓库静态检查工具谁都会配但很多人踩了一个坑全量跑动不动几千个历史遗留告警新人提交一行代码被淹没在噪音里。open-code-review 的做法是增量检查只检查当前分支相对主干的差异部分。比如我用 ESLint 的场景核心逻辑大概是这样的// review-diff-eslint.mjs import { execSync } from node:child_process; const changedFiles execSync( git diff --name-only --diff-filterACMR origin/main HEAD ) .toString() .trim() .split(\n) .filter((file) file.endsWith(.js) || file.endsWith(.ts)); if (changedFiles.length 0) { execSync(npx eslint ${changedFiles.join( )} --max-warnings0, { stdio: inherit, }); }把这段脚本接到 CI 里PR 的静态检查就只覆盖本次改动。历史遗留问题不会跑到当前 PR 上刷屏reviewer 收到的告警永远和本次 diff 强相关。增量检查还有一个隐性好处提交者本人更愿意修因为报出来的问题都是我的问题不是祖传代码的问题。3.3 用 Danger 给 PR 写机器人开场白Danger 是一个专门在 CI 里解析 diff 并留下 PR 评论的工具。我用它写了几条自定义规则让每个 PR 一进来就自动做三件事检查 PR 描述是否完整背景、测试说明、影响范围是否为空。评估改动规模超过 300 行自动提示建议拆分超过 500 行打 warning 标签。检测是否有 TODO、FIXME 被合入主干以及是否存在调试日志残留。一个 hook 的代码大概长这样// dangerfile.js import { danger, warn, message, fail } from danger; const { pr, commits } danger.github; if (!pr) { warn(这是草稿 PR请在准备就绪后标记 ready for review); } if (pr.title pr.title.startsWith(wip)) { warn(PR 标题包含 WIP合并前请移除前缀); } const linesChanged pr.additions pr.deletions; if (linesChanged 300) { warn(本次改动规模达到 ${linesChanged} 行超过 300 行强烈建议拆分为多个 PR); } const labels danger.github.issue.labels.map((label) label.name); if (!labels.some((label) label.startsWith(reviewed))) { message(自动标记此 PR 尚未通过完整人工审查); }Danger 评论的频率必须控制否则会变成狼来了。open-code-review 的原则是message 级别的评论只允许出现一次warn 可以有两次fail 直接阻断合并。机器人的价值是提醒不是刷存在感。3.4 敏感信息扫描这条安检线不能省老话说得好出事的时候都是慌到不行。密钥提交到仓库然后撤销很多人以为 GitHub 的提交历史删了就没事了实际上已经泄漏出去的 secret 必须立即轮换。与其事后补救不如在入口处拦住。gitleaks 在 CI 里的配置非常简单- name: 检查敏感信息 uses: gitleaks/gitleaks-actionv2 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}它会扫描整个 PR 的 diff如果发现疑似密钥、Token、私钥的字符串就会让状态检查失败。我建议不只看 PR 本身还可以定期对仓库全量历史做一次深度扫描dual check 才比较稳。3.5 自动化只处理可判定的问题把不可判定留给人类有个现象我反复见到团队上了自动化之后reviewer 开始有反正 CI 会拦住我随便看看的心态。这是不对的。自动化虽然能挡住 30% 的低级问题但它绝对无法替人回答NoSQL 缓存与数据库的一致性怎么做这个接口的权限设计是否越界这类问题。所以在 open-code-review 的使用说明里我一直强调一个原则自动化做筛选人做判断。CI 全绿不等于代码合格它只是说明代码没有踩中已知的坑。reviewer 必须明确自己看代码的重点是那些没有标准答案的部分。4. 检查单不是摆设如何把审查标准落到每个 diff 上4.1 为什么大多数团队的 check list 没人用几乎每个团队都有一份 code review checklist但绝大多数沦为摆设。原因很简单写得太抽象。请检查边界条件这句话说了等于没说。哪个地方有边界条件怎么检查检查到什么程度算通过没有可操作性人就不会用。我把这个现象总结为检查单的粒度诅咒太细了几十条看不过来太粗了没有指导价值。open-code-review 的解法是按 diff 类型分类不同场景激活不同的检查单而不是所有人都面对同一张大而全的表。4.2 按变更类型拆分检查单一次只展开一组我在仓库里内置了四套检查单模板Feature 类新功能、新接口。Bugfix 类修复线上问题。Refactor 类调整结构、不改外显行为。Migration/依赖类数据库迁移、升级依赖包。以 Bugfix 类检查单为例实际长这样复现路径是否写清楚修复是否覆盖了根因而不是只堵了表面有没有引入新 bug特别是对原异常路径的改动是否会影响正常路径线上验证了吗是否有监控数据或者日志可以佐证修复生效是否补充了回归测试回归测试是否能在修复前失败、修复后通过Feature 类的检查单则更关注边界、兼容性和权限设计输入校验完整吗枚举值、空字符串、超长串、特殊字符都覆盖了吗新接口是否做流量评估是否需要限流、熔断、降级输出结构变更是否影响老调用方是否存在兼容期异常分支是否都有日志和监控指标这套东西用 Markdown 挂在仓库docs/checklists目录下。reviewer 打开 PR 时根据变更类型把对应检查单过一遍然后在 PR 里回复一份勾选结果。不需要把每条检查项都复制到评论区只需要标注按 Bugfix 检查单核对完毕其中第 3 项存疑讨论就能聚焦了。4.3 安全审查最容易漏、也最不该漏的一项普通业务代码里安全审查是可以和正确性审查合并在一起的但我在 open-code-review 里把安全单拎出来因为它容易漏而且后果严重。三年前我们线上出过一个问题查了下日志发现某个内部接口把用户 ID 当输入直接返回了操作记录完全没做越权校验。事后复盘代码本身没有技术难点纯粹是审查时没人往这个方向想问。现在的安全相关检查单我强制要求涵盖这几项输入是否来自用户直接可控是否做了类型、白名单、长度校验是否有路径穿越风险文件上传的后缀和内容是否双重校验SQL 语句是否使用参数化查询有没有拼字符串的场景日志里有没有打印敏感字段手机号、邮箱、身份证号是否脱敏新接口是否需要鉴权角色权限是否在服务端做了校验而不是前端隐藏按钮如果你觉得团队安全基础薄弱这条检查单的落地优先级最高。它每抓住一个问题可能就避免一次线上事故。4.4 检查单要跟着线上故障持续迭代检查单不是拍脑袋写出来的它是从事故里养出来的。我每次线上出故障复盘完第一件事就是问为什么 review 阶段没发现如果确实存在可检查项就把这条回填到检查单里作为 Checklist 的下一版本。举个例子某次线上出现缓存穿透N 个请求同时打到数据库。复盘后发现代码里用了先查缓存没有就去查库的标准写法但忘了加分布式锁。这个经验沉淀到 Feature 类检查单新增一条是否在多实例并发场景下考虑缓存重建的并发控制。检查单有了这种来源团队信任度才会逐步建立因为它不是上面压下来的任务而是自己总结的避坑手册。5. 推进落地时踩过的坑和团队适配经验5.1 坑门禁一次性全开团队反弹到爆第一周我把 open-code-review 跑在核心仓库上把四道门全部打开强制 2 个 approve、ESLint --max-warnings0、gitleaks 扫描、500 行警告。结果当天下午就有同事在群里说合个代码这么费劲。第二周开始有人开始绕过系统先创建空 PR 让机器人过一遍再把大段代码直接 force push 进去。这是我犯的第一个错误流程的推进不能一次性到位要分阶段灰度。后来我收敛了方案分三步走第一周只上静态检查和 gitleaks不上 approve 门禁。让大家感受机器人帮我挑错的甜头。第二周加 danger 提醒和行数检查开始引导 PR 写清楚描述。第三周才开启 approve 门禁和 unresolved thread 不允许合并。每一层新规则上线前我在周会上花十分钟演示一遍让大家看到规范具体长什么样。灰度三周下来抵触情绪基本消退了。5.2 坑reviewer 没人愿意当还是轮值 贡献双轨制强制要求 2 个 approve 之后面临的核心痛点一下子从没人管变成没人愿意管。小团队的 code owner 就那么两三个所有 PR 都往他们头上挂谁顶得住后来我采用轮值 贡献双轨制来解决每周指定一名当值 reviewer负责该周所有 PR 的首次响应和 24 小时兜底。同时根据 git 提交历史自动推荐最有话语权的 reviewer。不管哪种方式approve 都必须给出一个实质评论不能只点按钮。轮值制把人从无限责任中解放出来核心 reviewer 不需要每时每刻在线只需要在自己当值的那周集中精力。这个机制跑了两个月PR 首次响应时间从 26 小时降到 4 小时以内。5.3 坑机器人评论刷屏人直接无视Danger 的 hook 里我一开始写了很多 message 类评论比如感谢你的贡献这个文件最近改动频繁建议关注之类。出发点是好的但效果很差——PR 里评论一多人的视觉焦点就散了真正重要的 warning 反而不突出。经验是机器人的每一次评论都必须有成本意识。open-code-review 现在的规则是message 级评论直接不要只有对合并有实际影响的 warn/fail 才发。非阻断的建议全部折叠到单个 issue 的 review thread 里不打扰主线程。5.4 坑小改动也被强制等两个 approve开发效率崩了一刀切要求所有 PR 都要 2 个 approve对大型 PR 合理但对改了 3 行的拼写错误这种改动就是灾难。于是我给规则加了一个分级改动少于 30 行的文档/配置/格式修复1 个 approve 即可涉及核心路径、数据库、权限模块的改动无论多小都必须 2 个 approve。这个分级需要在 Branch protection 里按路径配置GitHub 支持通过 CODEOWNERS 和 ruleset 组合实现。虽然配置的时候稍微麻烦一点但带来的效率提升非常明显——小改动不再卡流程大改动依然被严格约束。5.5 不同团队的适配参数直接抄这张表open-code-review 在文档里给了一套默认参数但每个团队都可以调。我认为最关键的参数有六个参数初创团队5人中型团队10-20人大型团队30人最少 Approve 数122核心库 3改行数告警阈值500 行300 行200 行首次响应时限48 小时24 小时12 小时超时升级时间72 小时48 小时24 小时增量静态检查开启开启开启自动合并小改动可以视情况需要审批表格只是引导真正要落地的时候我建议你从现在团队最痛的那个点下手。如果你最痛的是没人 review就先上角色和超时机制如果你最痛的是质量反复出问题就先上静态检查和检查单。没必要一上来把整套系统全部塞给团队。最后再分享一个我在推进 open-code-review 过程中感受最深的小技巧不要总在流程本身上面做文章而是想办法让流程给大家省时间。比如静态检查自动拦截拼写错误、gitleaks 挡住密钥泄露、Danger 自动检查 PR 描述——这些自动化省掉的是 reviewer 的低级重复劳动省下来的时间才有人愿意用来看真正重要的架构问题。流程只有在大家觉得它替我挡了很多事时才会真正活下去。
