从人肉评审到自动审查：代码质量门禁的落地实践

团队里PR评审越来越流于形式的时候我就意识到光靠人盯人肯定不行。代码审查这个事本意是好的但真到了几十个PR堆在队列里、每个人还要赶自己迭代的时候根本没人愿意逐行读别人的diff。后来我们开始做“open-code-review”这套自动审查体系把规则、脚本、工具链全部摊开从本地提交到CI门禁再到事后分析能自动化的绝不让人肉去扛。这篇文章就把我搭建这套链路时踩过的坑、用过的配置、以及为什么这么设计的思考全盘分享出来适合正在头疼代码质量和评审效率的团队参考。1. 整体设计思路把“人肉评审”变成“人机协作”1.1 为什么必须先让机器跑一遍先说一个扎心的事实人的注意力是有上限的。一次PR里如果超过400行diff评审者能真正仔细看的部分可能只有开头那几十行后面的基本就是“扫一眼觉得没毛病”。但恰好很多低级错误比如放了调试日志、临时改的测试用例没还原、变量命名前后不一致、新加的依赖没有lock文件——全都堆在没人仔细看的那部分里。自动审查解决的就是这个“低级错误漏网”问题。它不替代人去做设计判断而是先把机械性的检查全部挡掉。机器查过的点包括语法错误、未使用变量、明显不符合团队规范的地方、潜在的危险函数调用、过大的圈复杂度。这些检查如果靠人逐行去找既枯燥又容易漏但交给脚本和工具几秒钟就能出结果。我把这套体系定位成三层第一层是本地提交钩子拦下最蠢的错误第二层是CI流水线里的静态检查和安全扫描负责统一标准第三层是AI辅助审查和事后度量解决“风格之外”的问题。这三层各管一段既不让开发者在本地被烦死也不让CI成为摆设。1.2 分级而不是一刀切很多团队做自动审查容易走极端要么规则全开稍微有点不符合规范就报错要么干脆不管全靠最后人工Review。这两种我都试过效果都不好。规则全开的问题是噪音太多。ESLint刚接进来的时候我直接把当时社区里最热门的规则集全量引入结果第一个PR就报了60多个warning其中有一半是“多了一个空行”这种跟代码质量毫无关系的。开发者被这些无关痛痒的警告一刷就会养成“看到warning就跳过”的习惯真正重要的错误反而被淹没了。所以我后来坚持一个原则能阻断合并的一定是必须修复的问题其他的全部降级为提示。比如语法错误、明显的未定义变量、安全问题这些必须error级别而缩进风格、引号偏好这种交给格式化工具自动处理就好根本不需要人来管。这个思路直接决定了后面所有配置的写法。1.3 工具链选型的底层逻辑选工具之前先想清楚一个事你的团队用的什么语言栈这个工具是不是大家平时开发本来就接触的东西为什么这个问题重要因为自动审查工具是有学习成本的。如果你让一个Java团队去配一套复杂的分级审查框架大家第一反应一定是“这玩意怎么这么难搞不如我自己看”。但如果只是让他们在IDE里装上ESLint插件那几乎是零成本。我当时的选型逻辑是语言生态自带的Lint工具打底加上Prettier统一格式化配合EditorConfig统一缩进和换行再用Git钩子把检查串到本地提交动作里。CI层面用静态分析工具加上代码检视机器人最后是AI辅助的Review工具。这套组合的好处是每层职责清晰互相不打架。后来在另一个项目里我甚至只用了一个已有的代码扫描服务配合Github Actions里的几个现成Action就完成了最基本的自动审查闭环说明工具不在多关键是接得顺。2. 落地实操本地检查阶段的完整配置2.1 提交前的最后一道闸Git钩子与暂存区检查代码从开发者手里交出去之前最好能在本地就把问题拦下来不然等CI跑一圈再回来改来回成本太高。我用的是husky配合lint-staged的组合。这个组合的核心思路是只检查你即将提交的那些文件而不是整个项目的所有代码。为什么要专门提这一点因为我见过太多团队把检查做成了“全量扫描”一个大项目随便跑一次就要几十秒开发者提交一次等半分钟谁受得了lint-staged会读取git暂存区里面准备提交的文件列表只对这些文件执行检查和格式化速度会快很多。而且它能顺便把格式化的结果重新加回暂存区避免出现“提交的代码和我本地格式化后的代码不一致”这种诡异状况。配置其实很简单package.json里加上这样一段{ husky: { hooks: { pre-commit: lint-staged } }, lint-staged: { src/**/*.js: [eslint --fix, prettier --write], src/**/*.css: [stylelint --fix, prettier --write] } }这段配置的意思是当你执行git commit的时候pre-commit钩子被触发lint-staged会把暂存区里匹配到的文件拎出来先用eslint修一遍再用prettier统一格式。整个过程只需要一两秒或十几秒而且是项目只检查本次提交的文件体感上非常轻。还有个细节这两种工具都有--fix参数尽量加上能自动修复的问题就别让开发者手动改。2.2 规则分级怎么写才不会被开发者吐槽ESLint的规则分三个级别off、warn、error。我强烈建议在项目的早期就明确约定error级别的规则数量要克制但一旦设为error就必须遵守warn级别可以有但数量也要控制否则就是刚才说的噪音污染。拿我自己一个React项目举例我最后沉淀出来的核心配置长这样// .eslintrc.js module.exports { rules: { no-console: warn, // 允许用console但提示留意 no-debugger: error, // 调试器不能留在代码里 no-unused-vars: [error, { argsIgnorePattern: ^_ }], react-hooks/rules-of-hooks: error, react-hooks/exhaustive-deps: warn, }, };no-debugger设成error不用解释谁也不想把debugger留到生产环境。no-unused-vars设成error是因为这种代码基本都是忘删的留着就是垃圾但允许你用_开头来命名那些故意不用的参数。react-hooks/rules-of-hooks这个必须error违反它说明Hook用错了会引发运行时问题。warn级别的规则我倾向于留那些“程序员应该自己判断”的内容。比如react-hooks/exhaustive-deps它提示你依赖数组可能不完整但有时候业务就是要这么写所以提示一下就行不阻断合并。2.3 格式化冲突一个八十人团队都会踩的坑格式化工具和Lint工具职责要分清。Lint管代码质量和潜在错误格式化管风格统一。两者如果都去管同一个问题就会出现“Lint说这行太长格式化又给你改成另一种风格”的冲突。解决方式很简单让格式化工具优先跑在Lint前面并且把ESLint里跟格式相关的规则全部关掉。具体的做法是在ESLint配置里继承eslint-config-prettier它会把所有跟prettier重叠的规则一次性关掉。我自己早期没注意这个问题结果是每次提交代码eslint报格式错误prettier又自动把格式改掉两个工具在CI里互相矛盾导致提交被反复打回。最后查了半天才发现是规则冲突。后来统一成“prettier先格式化eslint再检查逻辑层面的问题”整个世界清净了。另一个容易踩的坑是Node版本变化导致的钩子失效。有次同事升级到新Node版本husky钩子直接失效了所有检查形同虚设。后来折腾半天发现是husky版本与Node版本兼容性问题我们固定了Node版本并在项目里加了.nvmrc才算稳定下来。3. CI流水线门禁的搭建与加速技巧3.1 哪些检查必须阻断合并哪些只需要看一眼CI流水线里的检查项如果设置得太多会让每次提交都变得很慢如果设置得太少又起不到门禁的作用。我建议把检查项严格分成两类阻断类和提示类。阻断类的标准是这个问题如果流入主干会直接造成线上故障、安全漏洞或者严重性能问题的。比如代码编译失败、单测失败、潜在的安全漏洞扫描异常。这类必须阻断合并没有任何商量余地。提示类的标准是这个问题如果存在后续会逐渐积累技术债务但不会立刻爆雷的。比如新增代码的单元测试覆盖率不达标、CodeQL报了低危告警、某函数圈复杂度偏高。这些可以让CI显示为提示状态人工Review的时候参考一下就行。这么设计的核心原因是每个PR只能有一个阻塞状态。如果所有检查都是阻塞的任何一个检查的误报都会卡住整个团队的交付节奏。而那几个真正重要的阻断项反而会因为“狼来了”效应被大家忽略。所以阻断项只能留给最核心的少数。3.2 静态安全分析接入不能只靠Lint普通的Lint工具关注的是代码风格和明显错误对于安全漏洞的检测能力其实非常有限。所以我建议在CI里加一层专门的静态安全分析。拿JS/TS项目举例可以接CodeQL或者npm audit这类工具。CodeQL能看到的数据流分析级别比Lint高得多比如“用户的输入拼进了SQL查询语句”这种模式普通Lint根本发现不了但CodeQL可以给你报出来。一个典型的CI配置我用的是GitHub Actions直接拉现成的CodeQL Actionname: CodeQL on: push: branches: [ main ] pull_request: branches: [ main ] jobs: analyze: name: Analyze runs-on: ubuntu-latest permissions: security-events: write steps: - name: Checkout repository uses: actions/checkoutv3 - name: Initialize CodeQL uses: github/codeql-action/initv2 with: languages: javascript-typescript - name: Autobuild uses: github/codeql-action/autobuildv2 - name: Perform CodeQL Analysis uses: github/codeql-action/analyzev2这个配置本身很简单但接入之后要有心理准备第一次扫描会报出一堆历史遗留问题跟新代码没关系。别慌这时候可以给CodeQL加上时间过滤只关注本次改动涉及的文件或者先只把新问题作为提示项等有时间再批量处理老问题。安全分析的目的是让新代码不再引入新的风险而不是指望它一夜之间把历史债务全部还清。3.3 CI速度优化的三个关键动作自动审查最大的敌人是慢。如果一次CI要跑十几分钟团队的效率就会肉眼可见地下降。我自己优化过几次总结出三个最有效的动作增量检查、缓存依赖、并发任务。增量检查的意思是只对本次变更涉及的文件跑Lint和类型检查而不是全项目扫描。这个在本地阶段用lint-staged做了在CI阶段也可以如法炮制用git diff来限定文件范围。缓存依赖很简单npm install或yarn install这一步的结果缓存起来只要package-lock.json没变就直接复用之前的安装结果能省掉好几分钟。并发任务更好理解把Lint、单元测试、构建、安全扫描拆成独立的job并行跑总耗时就从“所有任务之和”变成了“最慢的那个任务”。这里还要提醒一个细节CI跑Lint和格式化检查时最好直接跑那个只检查改动的命令不要整包扫描。可能有小伙伴觉得全量扫描才保险但实测下来全量扫描每次多花三到五分钟而这几分钟足够让开发者选择“先提交再说CI报错再改”的投机路径审查看似严格实际成了摆设。4. AI辅助审查与人工评审的分工边界4.1 AI能做模式识别做不了业务决策代码审查领域也开始有AI工具出现了比如CodeRabbit、Greptile还有用OpenAI API自己封装的一些Review脚本。它们的能力边界在哪呢我的体感是AI非常擅长模式层面的识别比如重复代码、明显不符合逻辑的写法、缺少错误处理、测试用例写得太弱等等。这些它看多了能从概率上给出有价值的反馈。但AI对业务意图的理解是有限的。比如一个字段从A接口挪到B接口在AI看来可能只是一次简单的移动但它不知道这背后是因为下游服务要重构是故意这么干的。如果AI给出“这个字段不该动”的建议开发者反而要花时间解释。所以我的原则是AI的建议全部作为提示不自动阻断合并人工Review才是最终的兜底。4.2 哪些场景必须保留强人工Review即使有了全套自动审查我依然会强制某些场景的人工Review不可省略。第一类是新的对外API接口设计。接口一旦发布就非常难改它的字段命名、返回结构、错误码定义都需要人来讨论。第二类是涉及权限、账单、数据删除这类安全敏感的逻辑。自动工具只能告诉你“这块代码可能存在问题”但具体业务上是否允许这样操作必须由懂业务的人拍板。第三类是大规模重构。比如把一个类的实现换掉工具只会告诉你功能测试是通过的但设计上是否合理需要资深工程师来判断。4.3 一次具体接入AI审查的实测记录我试过用一个开源AI代码审查工具接入到项目的PR流程中。刚接进来那会它会在每个PR下面留十几条评论其中大概有一半确实有用比如“这个函数第3行到第10行有重复逻辑可以抽取公共方法”“这个fetch请求没有catch错误会被吞掉”之类。这些问题我们之前确实没注意到于是顺手就改了。但另一半评论就属于瞎操心了。有一次它给一个正则表达式标注“性能风险”说这个表达式可能发生严重的回溯。我点开一看那段代码正则匹配的字符串长度最多不超过20个字符完全不存在性能问题。要是听AI的改成一个更复杂的写法反而增加理解成本。这块我从那以后就坚决把AI的评论标成提示只做参考。所以我的体会是AI审查适合当成“第一轮Review的预筛”帮你把明显的笨拙写法和潜在风险圈出来给真人Review提供方向但千万别让它直接说“批准”或“拒绝”合并业务层面的判断必须留给团队里最了解上下文的人。5. 常见问题与排查技巧实录5.1 高频问题速查表我整理了搭建这套审查链路的常见问题对照着排查基本能解决大部分情况。症状可能原因解决方式本地提交时不触发检查Node版本变动导致钩子失效固定Node版本重新安装依赖Lint和格式化互相冲突两个工具规则重叠继承eslint-config-prettier关闭重叠规则CI里Lint检查通过但本地报错本地和CI的Node版本或依赖不一致锁死package-lockCI和本地用相同Node版本安全扫描报出一堆历史问题首次接入历史债务集中暴露用时间过滤只关注新改动老问题单独建任务AI审查评论太多影响效率规则阈值设置过松只保留“确定性高”的提示比如安全问题和明显的重复代码CI整体耗时太长检查项没有增量化和并行化只检查变更文件拆并发job缓存依赖5.2 调试自动审查的几个好用小命令排查审查问题的时候有几个命令我现在几乎天天用。本地想看看这个文件改了之后Lint能不能过就跑npx eslint src/components/Button.jsx想看看格式化工具会把这个文件改成什么样但不真正写入加个--check参数npx prettier --check src/components/Button.jsx想看看husky钩子到底有没有被正确地装进来直接看.git/hooks目录下的文件ls -la .git/hooks/pre-commit如果这个文件存在而且指向的是husky说明钩子装上了如果不存在大概率是安装过程出了岔子。还有一种情况钩子文件在但内容依赖的脚本路径变了这时候重新执行一遍npm install或者yarn来修复即可。5.3 让团队从反感到接受的三个关键点自动审查上线初期团队普遍是抵触的抱怨“又被拦住了”“这个检查有啥用”。我的经验是要做三件事第一检查结果的信息要给得够好。错误信息里必须包含问题是什么、在哪一行、怎么修复。如果只有一句“违反了规则xxx”开发者还得去翻规则文档那不被骂才怪。ESLint报错会带链接我把这些链接整理成团队成员都能看懂的中文说明挂在团队的Wiki里。第二是区分配置的敏感度。有些规则看起来严格但实际经常报错这时候就要问自己这个规则带来的收益能不能抵消开发者的情绪损耗如果每次提交都被一个特别刁钻的规则卡住团队的效率会明显下降。不如把它降为warn让人工Review来判断。第三是让开发者感受到自动化审查确实帮他们省了事。比如自动格式化以前最烦的“代码风格不统一”问题被一键解决之后大家对lint-staged的接受度会明显提高。再有就是AI审查提供的建议能帮大家发现自己没注意到的小瑕疵几次下来大家就会觉得这个东西是助手而不是警察。最后分享一点我的实际体会这套自动审查链路的搭建过程最核心的收获就是每次只增加一个规则或一个工具跑上几天观察团队的反馈和CI的噪音水平再决定是保留、调整还是去掉。千万别一次性全上那样会把自己和团队都淹没在规则的爆炸里。代码审查的本质不是让工具替人做决定而是让工具把人的注意力从无意义的机械检查中解放出来去思考真正重要的事情。现在每次看到代码合并的请求我心里都有底多了该被规则拦住的问题早就被拦住了剩下进入Review环节的都是值得花时间琢磨的内容。