用自动化机制重塑代码审查：从人工疲劳到CI验证的工程实践

1. review 变流水账是从“看过就算审过”开始的维护某开源仓库的项目常年跟各种 Pull Request 打交道。最让我头疼的不是三四千行的大改动——那种改动大家天然警惕会认真拆开看。真正渗漏的反而是那些改一行配置、加一个空判断、调整一段注释的“小改动”。它们看起来太无害了审查者瞄一眼回复一句“LGTM”三四个人就这么放过结果上线后日志里全是新的报错。后来我也反思自己为什么在小改动上会放松核心原因是默认了小改动风险低于是没有设计任何针对性的防线。这就是 review 变成流水账的根源——我们审查的不是改动本身而是审查者对改动“感觉安全”的程度。这种感觉极不可靠。open-code-review 这个项目想做的第一件事就是重新划定审查基线。它把“看过”和“审过”明确拆成两件事看过是靠眼睛扫描一遍 diff审过是在改动被接受之前满足一套可以逐条打钩、可以被 CI 验证的条件。前者是主观的后者是客观的。1.1 把“看过”和“审过”分开open-code-review 的第一版规则非常简单只定了一条铁律任何改动在进入主干之前必须能在 CI 里找到三条与之对应的验证记录。改的是逻辑跑一遍测试并上传覆盖率。改的是依赖锁文件变更 diff 里能看出升级了什么。改的是文案或者配置至少要有一个静态检查的通过记录。没有对应的验证记录即使贡献者人缘再好、改动看起来再优雅也一律不能 approve。这个原则听起来不近人情但它恰恰是把“人工审查”从疲劳战里解放出来的关键。因为一旦你要求验证记录开发者自己在提交之前就会先跑一遍检查否则 PR 发出来被卡住走完一轮 CI 再回来改时间成本远高于本地跑一遍。于是很多低级的格式问题、漏参数问题、错误拼写问题都会在这一步被压下去。人工审查不再需要盯着变量名较劲而是可以把全部精力放到真正的逻辑风险评估上。1.2 流程先行而不是指望每个人都自觉团队里如果有一个人习惯了严谨 review没有人会觉得这是负担。但 review 真正贬值往往是从个别成员开始“快速通过”开始的。只要有一两个人开始划水其他人马上会觉得人家都过了我还审这么细干嘛这种默契一旦形成整个仓库的 review 质量会在很短时间里崩掉。我一开始试图靠沟通解决——开小组会强调审核纪律让大家深刻认识到问题严重性。效果有但维持不了多长时间。人一旦进入忙乱的节奏第一个被牺牲掉的就是这种需要持续自省的管理动作。所以 open-code-review 第二个原则是能用工具强制约束的就不要靠人自觉。代码提交前用钩子卡住明显问题CI 加上自动化检查作为红线这些约束一旦建立就不再依赖每个人每天都保持高压专注状态。这才是能让流程长期稳定的路。2. 把“审过”变成一个能在 CI 里验证的基线那“审过”具体包含什么为了不让这条规则变成一句空话我在 open-code-review 里沉淀了一份基础的 PR 检查清单任何一个仓库都可以直接抄过去改成符合自己项目语境的版本。### PR 提交前自检 - [ ] 代码能否在本地跑通所有相关测试yarn test / pytest / cargo test 等 - [ ] PR 描述里写清楚了 what改了什么、why为什么要改、risk影响面 - [ ] 涉及新增或修改接口时给出了调用示例 - [ ] 涉及 UI 或命令行输出时附上了截图或文本对比 - [ ] 依赖变更对应 lockfile 的 diff并说明了升级原因 - [ ] 文档、注释、示例代码已同步更新 - [ ] 所有自动化检查记录为绿色lint、类型检查、测试、构建可能有人觉得里面每一项都像行政流程。但仔细看每一项其实都在帮你降低沟通成本。就说“写清楚 why”这个要求对写代码的人是最便宜的——改动本就是自己做的理由就在脑子里写下来用不了两分钟。但对审查者来说这可能是全部上下文来源。2.1 没有上下文的改动是最贵的改动举个例子。某仓库有一个方法原本是同步操作某天某个 PR 把内部实现改成了异步接口没变签名没变但类加载顺序变了导致一个隐藏的初始化时序问题。如果 PR 描述里只写“重构内部实现”审查者对着 diff 看半天也看不出为什么会这样改。他只能去私聊提问等回复再继续审查。一来一回可能两个小时就没了。而真正的风险还得等代码上线后由系统告诉你。open-code-review 的实践告诉我一个优秀的 PR 描述比评论区里来回辩论十条消息更有价值。所有值得上线的内容都值得花三分钟写清楚它为什么值得上线。三分钟的描述可能帮团队省下三小时的沟通和一次现场事故。2.2 这些规则真的会在二十分钟之内产生效果可能有人会担心引入一套清单会不会让流程变得很重我实测下来不会。因为这套清单真正卡住的是那些本来就经不起问的问题不是正常人的正常开发。正常人正常开发代码自己写的测试自己跑过改动点自己清楚填 PR 描述只是把脑中的信息倒出来而已。清单只会让这一步变得更顺手并不会增加额外负担。真正会难受的是那些想通过模糊描述把平庸改动混进去的 PR以及希望别人跟着自己感觉走、不记录任何上下文的提交。这些人少了之后仓库的整体节奏反而会快起来。所以“审过”这个词最终的可验证定义是改动满足清单所有项并且所有自动检查转绿。达成这个状态人工审查者才拥有立足点可以专注于语义层面的事。3. 机器先审自动化检查的顺序与配置实例把人为约束拆解开之后下一件事就是让机器先跑起来。很多团队在做自动化时有个误区一上来就堆了一堆 linter、格式化工具、类型检查、覆盖率门槛结果开发者每次提交都要经历一套漫长的等待慢慢地就开始绕过钩子最后配置变成了摆设。open-code-review 对自动化检查的排序逻辑很简单先跑判断最快的再跑成本和收益最值得的。这套顺序不是拍脑袋定的而是根据实际经验得出的。# pre-commit 配置示例open-code-review 参考实现 repos: # 第一优先级秒级反馈的检查 - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.5.0 hooks: - id: trailing-whitespace # 行尾空格秒级 - id: end-of-file-fixer # 文件末尾换行秒级 - id: check-yaml # YAML 语法秒级 - id: check-merged-conflict # 冲突标记检测秒级 # 第二优先级语言级静态检查 - repo: https://github.com/psf/black rev: 24.4.2 hooks: - id: black # Python 格式化毫秒到秒级 - repo: https://github.com/pycqa/isort rev: 5.13.2 hooks: - id: isort # import 排序秒级 # 第三优先级类型检查 - repo: https://github.com/pre-commit/mirrors-mypy rev: v1.10.0 hooks: - id: mypy # 类型标注检查稍慢但仍可接受 # 第四优先级只读测试不跑集成 - repo: local hooks: - id: fast-tests name: fast-tests entry: pytest -m fast language: system pass_filenames: false always_run: true3.1 为什么让 cheap check 跑在最前面我特别想强调这一点。很多团队把最重的检查放在最后出发点是“反正都要跑”但实际效果非常糟糕。举个例子如果 pre-commit 第一步就是跑全量测试一个改动提交可能要等几分钟。在这几分钟里开发者完全有可能转到别的任务上去。等到测试全绿了他又得切换回来重新进入上下文效率已经掉下去一截。更麻烦的是如果测试失败是因为一个很蠢的 YAML 配置错误那开发者刚才的等待纯属浪费。而先跑秒级的语法和格式检查出问题当场就知道改完再提交成本极低。顺序调一下整个开发体验会完全不同。open-code-review 的经验是按时间成本分层让“马上就出结果”的检查永远跑在第一位让“要等一会儿”的检查排后面让最重的 CI 全量任务最后兜底。3.2 不同自动化检查的收益对比我在整理这套参考实现时对若干常见检查类型做了一个对比评估这里列出来供参考检查类型单次耗时能拦截的问题误报率建议阶段格式与空白秒级风格噪音、冲突标记残留极低本机 pre-commitimport 排序与 lint秒级到十几秒依赖引用错误、未使用变量低本机 pre-commit类型检查十几秒到数分钟类型流动导致的隐晦崩溃中低pre-commit CI单元测试快速子集数十秒到数分钟逻辑回归中本机 CI全量测试 / 集成测试数分钟以上全局回归与联调问题中高仅 CI依赖漏洞扫描数分钟标准运行已知 CVE 的依赖中高需人工核实仅 CI覆盖率对比与测试耗时相关未覆盖分支高不能用绝对值衡量仅 CI这套优先级不是完美答案但它解决了团队刚改造自动化时最常见的矛盾既想有拦截能力又不想把所有人拖进漫长的等待里。3.3 覆盖率门槛其实是陷阱聊覆盖率之前必须先泼一盆冷水用百分比数字作为硬性门槛大概率会催生刷覆盖率的行为。开发者为了保证数据达标会给本来就无关紧要的分支补一堆意义不大的测试代码库膨胀了覆盖率上去了风险却一点没降。我在 open-code-review 里用的是一个折中方案不设全局百分比门槛只做两件小事。一新改动涉及的模块覆盖率不得低于该模块历史基线的95%二关键函数标记了特定注解的函数行覆盖必须达到100%。这样可以防止新增代码出现大片未覆盖区域同时又不会逼着大家用无效测试堆指标。这个设计背后的逻辑很简单全局百分比适合宏观趋势观察但做不了代码合并的关卡。合并关卡必须盯着“增量”而不是“存量”盯着“本次改动有没有被测试到”而不是“整个项目整体覆盖率多少”。这个理念对任何开发语言都适用。4. 人只审机器查不出来的语义问题机器接管了格式、类型、低级逻辑错误之后人工审查的核心任务就变得非常纯粹了看语义看架构看改动能带来什么样的长期影响。怎么提高人工审查的效率 open-code-review 参考实现里有一个很直接的做法审查者按固定顺序读代码而不是随机浏览。第一个要看的是 PR 描述和 diff 摘要了解这次改动的意图。第二个看新增的测试用例。测试表达的是作者对代码行为的预期如果作者自己想的预期就是错的那测试会写得很别扭或者干脆缺失。第三个才进入实现代码本身逐步推进。4.1 一个有代表性的 diff 审查过程这里用一段简化的示例来演示人工审查真正应该关注的层次# 原实现 def send_notification(user, message): if user.is_active: gateway.send(user.contact, message) return True return False # 改动后 def send_notification(user, message, forceFalse): if not user.opt_out or force: gateway.send(user.contact, message) return True return False如果只是普通检查看到函数新增了参数 force逻辑从“是否活跃”变成了“是否退订或强制”看起来改动很小功能更灵活可能就直接放行了。但一个认真读语义的审查者会多问几句新增参数 force 是从哪里传进来的有没有可能是用户输入直接被透传成强制发送如果用户通过某个接口提交了 forceTrue那么退订用户的隐私选择是不是就被绕过了这个改动的安全边界在哪里这些问题没法靠 lint 或类型检查自动发现必须有人类审查者带着对业务上下文的理解去追问。也正因为如此机器检查得越充分人工越能从“找低级问题”里解脱出来去做这种真正有价值的纵深思考。4.2 审查标准的核心接口一致性、状态变化、失败路径我通常会建议刚接触严谨审查的开发者先盯三件事就够了接口一致性。改动有没有改变函数对外表现的约定参数、返回值、抛出的异常是否与文档和调用方预期一致。这是最容易被“内部重构”这类描述掩盖的问题。状态变化。改动是否引入了新的全局状态、类变量、缓存节点、文件读写并发场景下是否安全很多线上事故都来自隐藏的共享状态变化。失败路径。加了新的判断分支之后有没有对应的 else/except失败时是静默吞掉还是向调用方报错静默吞错是最危险的代码行为之一。这三条是代码评审里最能稳定复现价值的经验法则。你不必永远正确但只要每次都照着这个顺序问下去必然能问出有价值的问题。5. 从实际代码库中提炼出的五个高回报反模式open-code-review 参考实现整理到后期已经不再是技术配置问题了更多的是审视人的行为模式。我挑五个最常见的反模式这些是在多个项目里反复出现过的5.1 “LGTM 压力”当 PR 评论区已经出现一两条通过回复后最后一个审查者会天然降低标准。这是非常现实的社会学问题不需要指责任何人只要知道它存在就好。我的处理方式很粗暴项目里明确写道不允许使用“LGTM”作为唯一通过意见。通过的时候必须至少带一个实质评论可以是“通过确认可重入逻辑没问题”也可以直接指出一个非阻塞问题并存档。这个要求把通过行为从“短暂浏览”变成了“至少要说出一句与代码相关的话”。一句话的成本不高但它逼着审查者真的看了一眼内容。5.2 把风格偏好当成硬性规范审查里最容易伤感情的就是风格之争缩进用两个空格还是四个、变量名用 camelCase 还是 snake_case、if 是不是一定要带 else。这些东西如果让审查者逐个口头纠正会迅速消耗团队耐心。open-code-review 的做法是任何格式和风格层面的要求一律进入格式化工具的配置文件不允许出现在人工评论里。如果格式不正确机器会指出不会浪费任何人的口舌。只有当风格问题已经影响可读性而且确实无法用工具表达时才允许人工提出来。5.3 只看 diff 不看上下文很多代码审查工具默认只展开变更行审查者也只看这两三行这就容易漏掉“改动行之外”的影响。我见过一个经典案例某个改动只是在构造函数里加了一个参数校验本意是防御非法输入但构造函数被一个很底层的地方调用频繁触发校验异常导致整个模块直接无法启动。如果审查者只看 diff 区域确实很难发现这类问题。要避免这个坑就必须要求审查者至少打开改动涉及函数的完整上下文尤其要看调用方有多少、在哪里被调用。5.4 全员都审等于没人负责小型团队最常犯的错是规定“所有变更必须两个以上审查者通过”。听起来很严格实际执行时大家会互相匹配随便点通过审查责任被彻底稀释。open-code-review 的参考实现里是这样定义的每个 PR 只设置一个主要审查者他负有最终批准或拒绝的责任。其他人可以畅所欲言但不需要每个人都卡一道流程。一层明确的负责关系比三五个模糊的同意更可靠。5.5 把 review 拖成了“代码朗读会”有些团队喜欢聚在一起把一个 PR 从第一行念到最后一行每个人对每个细节发表意见。这种仪式感很足但效率极低。大多数人只是在等待轮到自己说话并没有真正消化内容。更合理的做法是异步审查加短小结论审查者自行阅读把问题一次性集中抛出来作者统一解答或修改然后回到同步讨论时只看分歧点。这个过程里时间用在刀刃上讨论争的是真正的分歧而不是把所有人的话平均听一遍。6. 小团队渐进式落地从一项检查到完整防线最后聊一下落地路径。如果你所在的团队只有三五个开发者或者你只是一个人在维护一个开源仓库告诉你“需要完整上一整套体系”是不现实的。open-code-review 的价值也恰恰在这里——它可以被切成碎片一步一步地引入。6.1 第一周只加一条规则第一周别贪多只做一件事在仓库里启用基本的静态检查随便选一个能跑的 linter。规则宁少勿多从一个能自动修复的配置开始。比如 Python 项目直接用 black 和 isort 自动格式化前端项目则直接配置 ESLint 核心规则。这一周的目的不是拦截多少问题而是让团队所有人都适应“提交时会自动过一遍检查”这个习惯。习惯建立起来之后后续再往上加什么都会自然很多。6.2 第二周要求 PR 描述具备三个字段下一周把 PR 模板里的三个字段设为必填what、why、risk。不需要长篇大论每个字段一两句话即可。但必须有没有就打回。这一步会明显改善审查者的阅读体验。审查者不再需要像侦探一样从 diff 里猜作者的意图而是可以直接读到作者自己归纳出的影响面。很多潜在的坑在作者填写的过程中就被自己发现并修掉了。6.3 第三周用机器固定住人工反馈把已经反复出现过的审查意见逐渐沉淀成自动检查。比如审查者第四次重复提醒“这个参数不该为 None”就立刻去写一个静态检查或者改类型定义让类似问题以后自动报出来。阶段里程碑可以参考这样的节奏第一个月本地钩子和 CI 静态检查全绿成为合并前提。第二个月PR 模板格式化完成上下文信息不再缺位。第三个月人工 review 开始只聚焦语义机器拦截所有低级问题。这套流程并不需要你们有一个专职的工程效能团队。它只需要一个认真的人愿意每周花点时间调整配置、回复意见、观察哪些问题再次出现。而 open-code-review 这个项目就是把那些认真的人积累下来的规则、配置、模板全部开源出来让后面的人不需要从头组织一遍。我自己在实际维护这个项目的过程中最大感受是代码审查这件事很多时候不是能力问题而是机制问题。机制设计得顺手普通团队也能做出高质量交付机制设计松散任你再资深的工程师也会在日复一日的快速通过中变得迟钝。自动化不是要取代人而是想把人的注意力从重复判断里抽出来留给真正需要判断力的地方。如果你也想在团队里推动一次这样的改造直接从一条配置规则开始就好不用等所有人同意先让机器替你说出第一句话。