★ FEATURED ARTICLE 上个月我在整理组里三个仓库的代码审查记录时发现了一个挺扎心的事实76% 的合并请求在合并当周内连一条有效评论都没有剩下那 24% 里不少还是“我这边没问题合并吧”这类走过场留言。你说我们缺流程吗审校清单、强制评审人、CI 门禁都有可大家就是有办法让这套流程变得像空气一样透明。问题不在流程在于流程没有载体审查意见散落在聊天记录里改没改没人跟踪反复出现的低级问题下个月再来一遍。我花了两周时间把这件事做成一个开源项目就叫open-code-review它的定位不是“智能审代码的机器人”而是一套能把审查规则、执行时机、结果回传串起来的工作流工具。它解决的是三个具体问题让机器负责那些能自动判断的审查项让人把精力留给真正需要判断力的设计讨论让每个审查结论都有据可查不再靠开会追忆让新成员拿到一份规则就能理解“这个仓库到底在意什么”。这篇文章把这个项目背后的设计取舍、核心实现和落地过程中踩过的坑完整写出来适合正在自己做工具链的开发者也适合想改进代码评审机制但不知道从哪下手的团队。1. 为什么写 open-code-review代码审查正在变成“走过场”代码审查这个环节奇怪就奇怪在越重要越容易被跳过。不是大家不知道它的价值而是它太依赖人的自觉。而只要一个环节依赖“自觉”它就会被交付压力第一个按死。1.1 口头反馈的三大黑洞我观察了一段时间后把代码审查失效的原因归结成三个黑洞第一个黑洞是评论即忘。评审人在即时通讯里给出一串“这里要判空”“那里命名不对”开发者当时回复“收到”改了但评论本身没有任何归档。两周后另一个模块出现了同样的命名问题谁也记不清之前讨论过什么。第二个黑洞是无主反馈。口头反馈经常没有明确的责任人更别说截止时间。一条意见发出来讨论区里都表示同意但没人去更新对应的代码。到了合并的那一刻这条反馈要么被忽略要么需要翻聊天记录才能想起来。第三个黑洞是经验不沉淀。团队里最有经验的核心成员每天在重复回答同样的问题日志别打到控制台、异常别吞掉、临时改动别提交。这些问题不是没有规则而是规则散落在各个人的口头习惯里新人唯一的学习方式是“被骂一次”。这些观察汇总到一块我得出一个结论审查效率低不是因为评论区没人说话而是因为该由工具自动判断的内容和该由人类讨论的内容混在了一起。机器只会做模式匹配但正因为模式匹配是可预判的它反而提供了让人无法拒绝的“确定性”。1.2 从“审查靠人”到“审查靠规约”传统审查的问题在不同人和不同时期标准不一致。今天某个参与者在场状态检查就严一点明天他请假同样的提交就能“便宜”过关。人在慎思也在变。open-code-review的出发点就是把那些可以写成正则、可以写成路径匹配、可以数出行数阈值的检查项从人的工作台挪到规则文件里。规则一经定稿就像给仓库立了一份“可执行的行为准则”。它不替代评审人的判断它替代的是“反复记忆规则”这件事。审查者看到的评论不再是一大堆“你这段代码有问题”而是“机器先完成它的例行公事剩下几条需要我动脑子”。比如我们团队最常见的一条规则是禁止把console/log一类调试输出直接推到主干分支。传统做法是核心开发者反复在评审里提醒现在规则文件里写一条 pattern命中即拦截。拦截不是最终目的目的是把人的注意力从“这种说了三遍的问题”里释放出来。1.3 这个工具适合谁不适合谁先说适合谁中小开发团队、独立开源维护者、想把评审文化从零搭建起来的小组。这类场景的特点是流程还没有僵化到不可改变工具能趁早切入形成习惯。独立维护者更受益因为被动的跨区协作能通过自动审查完成初步把关不用每一条贡献都必须等他亲手看完。不适合谁已经有一套成熟审查体系的大团队或者需要做全公司合规审计的场景。open-code-review没有做权限管理、审计追踪、审批流它专注在“发现问题并把问题变成结构化记录”而不是“谁来批准合并”。把这两件事混在一起工具会变成累赘。注意这不是一款“越用越智能”的审查工具。它没有机器学习没有“学你项目风格”的魔法。它的所有能力上限就是你的规则文件写得有多好。2. 工作流设计从一次 push 到一份结构化审查报告工具的名字叫open-code-review听起来像个大的评审平台但它真正的核心其实是个工作流引擎。整个过程可以总结为四个字触发、扫描、匹配、回写。2.1 触发链路的四个节点我在设计触发链路时首先列了一堆可以启动审查的时机开发者本地提交、推送远端、合并请求创建、合并请求更新、定时全量扫描。最后确定的原则很简单——宁可少触发不要滥触发。审查不是越频繁越好高频触发会导致评论噪音噪音会让用户习惯性忽略工具的提醒有实际价值的问题也会被连带跳过。最终采用了四个触发节点它们之间是串联关系本地pre-push在代码离开开发者机器前查一轮拦截那些最不需要讨论的低级问题。远端合并请求更新这是主战场给每一个待合入的变更生成审查快照。合并前门禁只做“阻塞检查”影响面控制在 release 构建之前。定时扫描可选针对历史遗留代码每周末跑一次把存量问题迭代清一点。每个节点都有自己的职责。pre-push要求快速尽量 5 秒内跑完合并请求更新要求全面宁可伤一点性能也要覆盖所有配置的规则门禁检查只关心结果的决策字段如果只有提醒级别的问题是不能阻止合入的。一次完整的链路大概是这样的开发者推送提交 → 本地钩子先扫一遍没问题放行 → 远端触发 CI 里的审查任务 → 工具拿到git diff按规则文件扫出命中项 → 把命中项按文件/严重程度分组 → 通过 API 回传到合并请求页面同时也写一份 markdown 报告保存到流水线产物里。2.2 审查配置文件的字段设计配置是open-code-review的灵魂。我不想把它做成一个只能通过代码扩展的工具——那样的话每个团队都得写一堆脚本维护成本太高。我更希望它像.eslintrc那样一个文件搞定所有规则改规则如同改配置不用动程序本体。下面是一个简化但完整的配置文件示例version: 1 output: format: markdown path: review-report.md global: # 默认忽略的目录任何规则都不会去扫描这些路径 exclude: - vendor/** - node_modules/** - dist/** rules: - id: no-debug-logger severity: blocker scope: diff files: include: - **/*.py - **/*.js pattern: console\\.log|print\\( message: 调试输出被推送到受保护分支请清理后再提交。 shortcut: drop-debug - id: todo-without-issue severity: warning scope: pushed files: include: - **/*.ts pattern: TODO|FIXME|HACK message: 发现未关联任务标记{match}。建议带上 issue 编号便于后续跟踪。 skip_merged: true - id: large-diff severity: suggestion scope: pull_request # 这个规则不走正则而是走内建的“行数统计器” kind: threshold threshold: lines: 500 message: 本次变更超过 500 行考虑拆分为更小的合并请求方便逐段审查。有几个字段我想特别解释一下scope控制这个规则在哪个阶段执行。diff表示只扫描本次变更行pushed表示扫描整个推送范围pull_request表示在合并请求级做全局判断。shortcut是规则的短名用于在命令行里快速临时跳过某个规则。有时候开发者会在本地跑一遍“除风格检查外的所有审查”这个字段就能派上用场。skip_merged表示如果这个 TODO 是合并请求中“未被修改的行”就忽略。这个字段是我后期加的为了镇压“老代码里的 TODO 被新代码误报”的经典问题。kind字段如果不填默认是regex模式匹配填了threshold就走内置的特殊判断器。2.3 为什么坚持“规则文件驱动”而不是硬编码检查器立项时我身边有两条路线一条是把几十个常见审查项直接写死在 CLI 里另一条是做成纯粹的插件市场。我都没选而是选了“规则文件驱动”这个中间态。硬编码检查器的好处是开箱即用坏处是很傲慢。它默认开发者能遇到的所有问题“已经被作者提前猜到了”。项目一复杂开始需要仓库独有的规则比如“这个模块不得 import 那个模块”“对新接口的调用必须加日志埋点”这些硬编码占不到。如果为了支持这些场景就继续堆代码工具会越做越臃肿。规则文件驱动的好处在于规则本身就是数据。你不需要改程序只需要改配置你可以把配置放回代码仓库里随版本演进做 review。更关键的是规则可以用模板字符串生成自定义提示信息甚至可以在message_template里嵌入匹配到的具体文本这样开发者看到的评论不是冰冷的“你犯错了”而是“这个函数里有一处 print 调用需要移除”。高信息量的评论更能让人接受。3. 核心模块拆解扫描、评论、报告三条主线配置定下来之后剩下的就是工程实现问题了。整个工具从代码结构上拆成三个模块扫描模块、评论模块、报告模块。三个模块中间通过一个标准数据对象传递格式大概是{ rule_id, file, line, severity, message }。3.1 审查清单扫描器先缩小范围再逐条命中扫描器是这个工具里性能压力最大的模块。一开始我实现的是“全库扫描”把整个仓库 checkout 下来然后在每个文件上跑所有规则。这种方式代码简单但有两个严重问题一是大仓库动辄几万个文件扫描一次要好几十分钟二是会产生大量“历史遗留问题”的命中与本次变更毫无关系。后来我改成“基于 diff 的增量扫描”思路很直接先从 git 命令里拿到本次变更涉及的文件列表和行号范围再把这些文件读进内存只对相关行跑规则。整个改动之后扫描耗时从几分钟降到了 3 到 5 秒。增量扫描的伪代码逻辑大致是def extract_diff_range(git_root, base_ref, head_ref): # 调用 git 获取差异信息解析出每个文件的增删行区间 # 返回 { file_path: [(start_line, end_line), ...] } result {} output run_git([diff, base_ref, head_ref, --unified0]) for change in parse_unified_diff(output): result.setdefault(change.path, []).append( (change.new_start, change.new_end) ) return result拿到行号范围后扫描器会跳过规则里exclude的路径再对剩下的文件逐行做正则匹配。为了进一步提速我对正则做了分组预编译同一组规则用的 pattern 在启动时统一编译一次循环里只做匹配不重复编译。这个模块有个很容易被忽略的边界问题新增文件没有 diff 区间需要在解析 diff 时额外识别new file mode标记走一遍全文件扫描。如果漏掉这个分支新增文件就永远不会被审查到。3.2 评论推送器增量、去重、可预览扫描模块产出的是一堆命中记录但直接把这些记录推送上去会引发灾难。最典型的场景开发者第一次提交工具丢出 30 条警告开发者改好一次工具又丢出 28 条警告。如果每次都把完整的命中列表同步到合并请求评论区评论区会变成一个不断刷新的“噪音场”。所以评论推送器的核心逻辑是增量推送 本地去重。工具会维护一个审查状态文件记录上一次已经推送过的命中指纹。指纹由rule_id file line message四元组拼接后做哈希生成。在这一轮推送前先跟上一轮比对只推送新增命中已经消失的命中不发“撤销评论”但会在报告的表格里标注“本次已解决”。这样做带来一个额外的好处合并请求的评论数量会随着迭代次数收敛。开发者修掉一个 warning下一轮评论里就不会再出现它。如果还在重复出现说明这处代码可能改错了位置问题值得关注。推送器还有一个 dry-run 模式所有结果只输出到 stdout不真正调用任何 API。我强烈建议落地时先开一个星期的 dry-run原因很简单规则文件一上来必然误报误报直接刷到评论区会让团队瞬间对工具失去好感。先让它在后台安静跑几天把误报调低再正式上线。3.3 报告生成与“审查密度”指标报告模块是我最喜欢的部分。它把每次审查的命中记录整理成一个 markdown 文件按严重程度blocker / warning / suggestion分组统计每个文件命中次数、每条规则触发频率并计算出一个我给它的指标——审查密度。审查密度的算法是审查密度 命中总数 / 变更代码行数 * 1000单位是“每条关于一千行变更的评论”。这个指标的妙处在于它消除了绝对数量的误导。一个 50 行的小改动出现 5 条警告看似不严重但审查密度高达 100一个 2000 行的大重构出现 20 条警告审查密度只是 10。前者才真正需要停下来看看代码是不是写得过于稠密。报告会额外输出一个 top10 命中规则列表这个列表对规则调优帮助极大。我见过的最夸张一次是某仓库前一周的报告中有 40% 的命中来自同一个规则——“变量命名长度低于 3 个字符”。这说明团队压根没在命名上达成共识过规则推得太超前而不是团队有什么严重问题。4. 两种落地方式本地 pre-push 与流水线合并请求审查工具构思得再好最终都要落到一个具体的触发方式上。open-code-review支持两种最常用的落地路径我分别展开说一下部署细节。4.1 本地 pre-push 接入推送前的最后一道闸门本地接入依赖 Git hooks。如果你只是简单复制一个脚本进去那跟大多数项目里的“一次性钩子”没什么区别因为 Git hooks 默认不会随仓库分发给所有开发者每个开发者的本地钩子都需要单独安装一次。所以open-code-review提供了一条用命令完成初始化的路径它会把钩子脚本和必要的辅助文件放进当前仓库的.git/hooks/pre-push并赋予执行权限。一个简化版的pre-push脚本长这样#!/usr/bin/env bash set -euo pipefail REMOTE${1:-} REMOTE_URL${2:-} # 只对 main 分支被推送时做本地拦截 CURRENT_BRANCH$(git branch --show-current) if [[ $CURRENT_BRANCH main ]]; then echo open-code-review: 正在本地审查 main 分支推送... open-code-review run --scope diff --block-on blocker fi这里的--block-on blocker表示只有blocker级别的问题才会阻止推送warning和suggestion只打印。这个策略很关键否则没人敢把本地钩子开着。本地接入最大的收益是反馈时延最短。开发者还没把代码推送到远端本地就知道哪里有问题。我实测下来只要扫描速度控制在 5 秒内开发者几乎感知不到它会拖慢速度而如果真拖慢到 15 秒以上大概率的结局就是使用者偷偷把钩子删掉。4.2 流水线合并请求审查把结果挂在“待合入”入口流水线接入是更主流的用法因为大部分团队都习惯让审查发生在合并请求环节。这一步你不需要在开发者机器上装任何东西只需要在 CI 配置中增加一个任务review-job: stage: test script: - open-code-review run --scope pull_request --base refs/heads/main --head $CI_COMMIT_SHA --output report.md --publish-comments artifacts: paths: - report.md在执行到--publish-comments这一步时工具会根据当前合并请求的变更内容在对应位置添加评论。这里的关键是如果 CI 系统用的是旧版 API评论无法直接指向某个具体提交行它就只能以“整份报告”的形式追加到合并请求底部。后者效果差一些但至少信息没有丢。另一个需要特别注意的地方是合并请求更新时旧评论不会自动消失。如果每次都重新发布全部评论评论区会越来越长。因此生产环境里我强烈建议同时开启--prune选项它会让工具自动关闭“上一发布批次中已经不存在的问题”对应的评论线程。效果略大于粗略更新但带来的阅读体验提升是决定性的。提示流水线接入时不要把blocker级别的检查直接做成“流水线失败”的硬门禁。建议第一阶段只把问题标记在合并请求页面上跑两周观察误报率等规则稳定了再逐步把硬门禁打开。直接上门禁理想很丰满现实经常是全体开发者在等着你把规则改对不然没法合代码。4.3 结果回写仓库的几种姿势与权限建议结果回写有三种典型姿势各有权衡。第一种是只生成报告文件作为流水线产物保存。这种方式最保守不会在贡献者的代码上涂抹任何东西适合最开始试水。缺点是报告和代码的关联是松散的开发者不会主动去翻流水线产物。第二种是评论回写合并请求我前端时间在用的方式。它把问题直接放在代码变更的上下文里开发者在合并请求页面就能看到上下文从审查体验上讲是最好的。代价是使用者需要一个具备“向合并请求写评论”权限的机器人账号而且这个账号的权限要严格控制只能写评论不能直接合并代码。第三种是把结构化结果写回一个 JSON 文件并提交到仓库的 meta 目录。这招适合定时扫描场景可以把每周的扫描结果存档形成趋势曲线。但它不适合日常工作流会产生大量合并提交异常干扰仓库历史。权限建议就一条永远别把机器人账号的权限开成仓库管理员。它只需要“读取代码 提交评论”这两个权限多了都是风险。出问题时最小化影响比方便更重要。5. 跑了两个月之后数据变化、踩坑与规则调优任何工具都是上线之后才开始经受真正考验的。这个项目在我们的三个仓库里跑了两个月效果有好的方面也冒出了不少值得记录的问题。5.1 噪音量是怎么一步步降下来的第一周数据是非常难看的。三个仓库加在一起总共在一周内产生了 58 条机器评论其中有 23 条属于成员公认的“废话”——要么是规则写得太宽把不该管的包了进来要么是历史遗留代码跟新代码混在一起被误扫到了。我不建议任何人直接删掉规则文件重来。正确做法是看每一条评论给它们打标签有效 / 误报 / 边界情况。然后把反复出现的误报归类看是正则写得太粗糙还是 scope 选错。最经典的案例是我们早期一条规则“禁止 console.log”结果把测试文件里的辅助打印也给拦了。修正方案是调整 files.include 字段排除**/*.spec.js一次干净解决。第三周开始噪音量降到每仓库每周 11 条第五周就只剩 4 条左右。有效评论的比例从最初的 34% 升到了 82%。这个数据变化其实说明一个道理审查规则的收敛速度取决于你有多愿意看评论反馈。规则不是写出来就完事了它是一个活体需要每周照镜子。下面是其中一个仓库三周的数据对照周次机器评论总数有效评论误报评论有效占比第 1 周58202334.4%第 2 周2315665.2%第 3 周119181.8%5.2 “审查密度”出来后团队行为变了审查密度这个指标刚开始只是报告里的一个数字后来却成了团队里一个挺有意思的“游戏化”参考点。我们默认把它算进每周的代码质量小结里主要用来找出“哪些改动需要优先人工 review”。有个现象很值得一说当开发者知道自己的改动会被计算密度时不少人开始主动在本地先把明显的console.log、误提交的调试断点、奇怪的临时变量清理干净。他们不需要被命令只是当机器会把“每千行评论数”亮出来自动去查而不是“被说”成了默认行为。这其实揭示了审查工具一个容易被低估的作用。工具不一定非要直接阻止问题它还可以通过制造可见的指标改变开发者的心理预期。如果每次推送都看到机器在盯着人就会先在心里过一遍自己的代码。这比任何强制的“审查会”都有用。5.3 几个容易翻车的小细节最后总结几个实际运行中容易翻车的细节希望能帮少走点弯路。第一个是Windows 与 Linux 的换行符差异。同一份代码在两种系统上 diff行号偏移会出现整行错位。工具在解析 diff 时必须统一按\n处理否则同样的代码在 Windows 用户那里会被误报“多删了一行”。第二个是编码识别。强制把所有文件都当成 UTF-8 读碰到含 BOM 的文件会在正则匹配时产生未知字符导致部分 pattern 匹配失效。应对办法是读文件时先判断前三个字节是否等于EF BB BF是则移除。第三个是大型二进制文件的保护。扫描器必须跳过图片、压缩包等所有二进制文件否则正则匹配会把二进制流里的偶然命中当成问题报出来。建议在路径过滤里加一层二进制扩展名白名单判断而不是依赖文件后缀。第四个是规则临时静默。在生产环境里一旦某条规则误报突然炸了你要能在 5 分钟内把这条规则临时关掉而不是花两小时改正则重新发版。我最终给 CLI 加了一个--disable-rule参数支持多值输入专门应付这种紧急情况。我自己的体会是规则文件驱动的价值恰恰在于它把审查的边界划得很清楚工具只做那些可以被明确表达、被版本管理、被反复验证的事情人做的事情则永远保留在那条模糊的边界之上。代码审查这个环节想做好从来不是要让机器比人更敏锐而是让机器的稳定性和人的判断力各自待在擅长的地方。下一步我会继续往可插拔检查器、增量缓存和跨语言规则集这几块做扩展如果你也在搭类似的流程试着先从最烦人的三条规则跑起来你可能也会发现团队愿意认真看审查意见的那一天比想象中来得更早。
阅读完成 · 觉得有帮助？
