我做代码审查差不多十年了经历过审查五分钟开会两小时的传统评审会也经历过每个PR都要等三天才有人理的线上协作但真正让我决定搞一套自己的开源代码审查方案的是2023年下半年的一次线上事故——一个本该在上线前被拦住的空指针问题因为在代码审查阶段被秒approve放行了结果凌晨两点被线上告警吵醒。团队里不是没有审查流程流程都有制度也都写了但就是没人仔细看。这让我意识到一个问题代码审查的真正难题从来不是有没有流程而是流程有没有被真正执行以及有没有一套东西能把人和人的注意力聚到最关键的地方。这个念头最终落地成了一个我到现在还在持续迭代的开源实践项目就是标题里的 open-code-review。严格来说它不是某个大厂出的重量级框架而是一套把开源审查工具编排起来、配合规则和流程规范的最小可用方案。它解决的是中小团队最常见的三个人痛点PR没人认真看、审查意见停留在格式修正层面、合并门禁形同虚设。全文不涉及任何商业闭环纯粹是从一线工程视角出发告诉你我从零搭这套东西的时候做了哪些选型、踩了哪些坑、最后沉淀下来哪些可以直接抄走的配置和习惯。如果你也受够了审查全靠自觉的日子或者你在纠结要不要引入代码审查工具链但怕太重这篇东西应该能帮你省下不少试错成本。1. 为什么我决定自己组建一套代码审查工具链1.1 团队里的三个审查死角很多人以为给GitHub或者GitLab配上了Protected Branch再装一个Dependabot或者SonarQube代码审查就算完事了。实话说这些工具单个拿出来都很好但组合在一起之后一般团队会碰到三个特别隐蔽的死角。第一个死角是审查意见停留在表面。在没有规则引擎介入的情况下人类审查者会把大多数精力花在看缩进、命名、有没有多余的空行这种低级问题上。不是说这些不重要而是当一个人药铺把80%的注意力耗在这些机械性问题上的时候真正需要人脑判断的逻辑漏洞、并发隐患、业务边界问题反而被漏掉了。我自己就吃过亏一次Pull Request里有400多行改动其中300行是格式化插件自动改出来的审查者翻了几页看到全是格式变化就不自觉地放松了警惕结果一个嵌套事务的边界条件被漏掉了上了生产之后偶发数据不一致。第二个死角是合并门禁是假的。很多团队的GitHub分支保护规则只设置了至少一个Approval但并没有限制谁可以审批也没有要求审查者必须不是提交者本人。于是经常出现的情况是张三提交了PR自己点了个Approve然后合并按钮就亮了。这种门禁不仅没有质量把关作用还会慢慢摧毁团队的审查文化——因为大家发现规则就是摆设。第三个死角是审查的知识没有沉淀。口头审查完、书面评论也提了但三个月后问起来大家都不记得当时为什么这么改、为什么拒绝那个方案。代码仓库里的commit message写得像日记fix bugupdate style这种占了一大半。正是这三个死角让我开始认真考虑一个问题能不能在现有Git托管平台的基础上加一层轻量的审查编排层让机械的事情自动跑让人的注意力集中在真正需要人判断的地方同时把每一次审查的结论结构化地沉淀下来这就是open-code-review这个项目最初的核心动机。1.2 商业SaaS和自建开源方案的取舍当时我试过几款商业SaaS的产品比如市面上比较成熟的代码质量平台。客观说它们的自动化分析能力和Dashboard都很成熟开箱即用的体验很好。但我当时有三个顾虑。第一是代码安全合规的问题。公司内部的代码不能随意往第三方云服务上送尤其是涉及核心业务算法的仓库。要找安全评审、签NDA、走采购流程那周期不是一两个星期能搞定的。第二是费用问题。商业平台基本按人头或按仓库数收费团队规模超过一定数量以后这笔钱其实不少。特别是一些内部工具仓库活跃度很低但按人头算一个不少性价比很差。第三是定制化不够灵活。商业平台的分析规则是通用样板对特定业务场景的支持往往要等官方更新。而一般的开发团队用的语言、框架、目录结构、commit规范差异极大通用规则往往有大量误报团队对平台的信任度降低之后就干脆不看报告了。相比之下自建基于开源工具的组合方案虽然要自己花时间去搭、去调但至少有三个明显的好处代码不出私网、成本可控、规则完全可以按团队自己的需求调。这也是open-code-review最核心的一条设计原则不追求一次性搞定所有事情而是先搭一个能跑通的最小闭环再不断迭代。1.3 open-code-review的定位轻量自动化审查编排层说了这么多我得先给open-code-review一个明确的价值定位。它不是要替代GitLab或者GitHub的Code Review功能也不是要替代SonarQube这种独立的静态分析平台。它更像是一个编排层——负责在提交PR或MR的时候把一堆已有的开源工具按照正确的顺序跑起来然后把各自的输出汇总成一份统一的审查报告再把这份报告附到MR discussion里面同时把结论同步给相关的人。这套方案里审查被拆成三个层次自动化层处理那些机器可以明确判断的问题——格式规范、潜在Bug模式、依赖安全、代码覆盖率变化。人工层负责处理自动化层给不出的结论——逻辑正确性、架构合理性、业务语义一致性。沉淀层将每一轮审查的结论、拒绝原因、修改要求记录下来生成索引方便后续复盘与检索。这个分割方式是一线团队最需要但往往最失控的部分。很多团队自动化层几乎为零把一切都压在人身上也有一些团队过于依赖自动化结果一屋子机器人提出一堆不痛不痒的comment人类审查者干脆全Ignore。open-code-review在设计的起点上就是想给这三层找到合理的配比。2. 工具链选型踩坑从Gerrit到GitLabReviewBot2.1 我对比过的几套方案在正式介绍我最终落地的技术栈之前有必要说说我在选型阶段踩过的坑以及为什么没有选择某几个非常主流的东西。第一套考虑过的是Gerrit。Gerrit在开源社区地位非常高尤其很多大厂内部用的都是它的某种变体它把Patch Set和Verified Code Review的分级流水线做到极致对Android这种大仓多补丁的场景特别合适。但如果只是给一个几十人的团队做PR审查Gerrit的问题是它需要把整个代码流转模式都改掉——它用的是基于Push的新Change模型跟GitHub/GitLab的Fork Merge Request模型很不一样。团队要重新学习一套交互模型迁移成本超出了我当时能承受的范围。第二套是GitLab自带的Merge Request Approvals和Code Quality功能。如果你的团队已经在用GitLab这套东西的普及成本最低开箱即用权限模型也成熟。但GitLab自带的能力偏流程控制而非审查辅助——它能告诉你这个MR需要一个人批准却不能告诉你这份代码里这个函数是不是写复杂了。我必须再叠加一层静态分析工具才能达到我要的效果。第三套是GitHub之上的一套组合eslint reviewdog GitHub Actions。这个组合在JS/TS项目里非常顺滑。Reviewdog这个工具尤其强大它可以把linter的输出转化为PR上的Inline Comment审查者在PR里直接看到这一行有这个错误。但它的问题是对多语言支持不够均匀我团队里还有Go和Python的仓库就得再搭一层。第四套是SonarQube。它是老牌的静态分析平台支持几十种语言有很强的规则定义能力和历史数据追踪能力CI集成起来也很方便。但它的部署和维护相对偏重需要一台单独的服务器跑PostgreSQLSonarQube实例而且规则配置项多到眼花缭乱。对一个小团队来说投入产出比不太划算。2.2 选型定下来的关键标准在把我自己手上的方案全部列在纸面上对比过之后我给选型定了几条硬标准。不是说每一套方案都要满足所有标准而是在这些标准上的综合分要最高。第一条是集成成本低压过纯流程改造。不能为了加自动审查而让团队改变原本的MR流程最好是原有提交流程不变只是在MR的合并条件上加一道门槛。第二条是审查结果必须落在MR本身。不能把审查结果导出到另一个系统里让开发者去查而是要以Inline Comment的形式直接出现在MR的Diff上。人都是懒惰的如果看一次审查报告要点三个不同的链接坚持不下来这个方案就没法落地。第三条是规则可配置且更新方便。团队的语言占比会变、代码规范会演进规则配置如果不能做到改一个配置文件就能生效会很痛苦。第四条是有扩展能力。不能把自己绑死在一个语言的工具链上。至少做到如果以后要加一个新语言的linter只需要写一个适配器而已。综合这些标准之后我最终选择了一主一备的架构主流程用GitLab CE GitLab CI配合Reviewdog 各语言的Linter和静态分析器再加一个我自己用Python写的小脚本把各工具结果合并成统一JSON结构最后通过GitLab API回写MR Discussion。有条件上也考虑过GitHub Actions的版本但由于我们主要仓库在GitLab上基本盘就定在GitLab了。2.3 整体技术架构这套open-code-review的方案架构你可以理解为三个部分触发层开发者在本地提交分支、发起Merge Request之后GitLab CI的Webhook会自动触发一个pipeline。执行层Pipeline里面有若干个Job每个Job负责一种检查。比如Go代码用golangci-lintPython代码用ruff和mypy前端代码用ESLint和Prettier依赖安全检查用Trivy或者npm audit。汇总反馈层每个Job跑完之后把结果转成Reviewdog的diagnostic格式由Reviewdog统一扇向GitLab最终以Inline Comment的形式出现在MR的Diff上。同时我写了一个很轻的Go语言二进制工具叫review-summary用来把所有Job的JSON结果汇总成一个Markdown报告包含总体结论、阻塞项数量、建议项数量、覆盖率变化等然后在MR描述里更新这个报告。这样审查者一打开MR第一眼就能看到哪些问题必须修哪些问题仅供参考。这个架构相比直接用一款全家桶式平台的好处是每一项都可以被替换。比如你不想用GitLab CI想换GitHub Actions那只需要把执行层的调度器换掉不想用Reviewdog想用代码弟弟自家的Bot都可以因为汇总层只认标准JSON格式。它像在流水线和工具之间加了一层适配保证了组织架构和代码仓演变时的灵活度。3. 落地部署的完整过程从空仓库到第一份自动审查报告3.1 先解决审查数据从哪来GitLab CI的事件接入第一步要做的是确定GitLab CI的触发条件。我建议你的流水线至少在三种情况下运行完整的审查合并请求发起或更新时、推送到主分支时、以及定时任务每日凌晨。触发方式在.gitlab-ci.yml里面配置workflow: rules: - if: $CI_PIPELINE_SOURCE merge_request_event - if: $CI_PIPELINE_SOURCE push $CI_COMMIT_BRANCH main - if: $CI_PIPELINE_SOURCE schedule这里有一个很关键的点不要在push到任意分支时就跑全套自动化检查否则大量临时分支也会触发流水线白白消耗CI资源。只在MR事件和main分支推送时跑是主流团队的标准做法。然后针对MR事件要拿到目标分支和源分支的名字以及MR的IIDMerge Request的编号。通过GitLab CI的内置变量就可以获得了比如CI_MERGE_REQUEST_IID、CI_MERGE_REQUEST_TARGET_BRANCH_NAME、CI_MERGE_REQUEST_SOURCE_BRANCH_NAME。这些变量会在触发器作用于合并请求时自动注入环境。如果你用的是GitLab Runner的Docker executor建议在Runner上配置好镜像拉取策略避免每次都重新拉几十GB的镜像。这个影响跑的速度但更重要是不要因为权限问题导致runner拉不动私有镜像。3.2 规则引擎的配置细节Linter不是装完就能用的这一步是整个方案中最花时间的部分被很多人低估。以Go仓库为例golangci-lint在默认配置下对大型仓不要说几十分钟都跑不完而且会有大量历史存量问题冒出来一屏红错误直接把开发者的积极性击穿。所以必须做一个动作先跑一次全量生成基线然后在规则配置里只对新增代码做严格检查对存量问题只提醒不阻塞。具体可以通过设置new-from-rev来实现linters-settings: new-from-rev: HEAD~1意思是只检查从上一个commit引入的新问题。这个配置能救你的命。强烈建议第一次接入任何linter时都优先考虑增量检查的模式除非你的团队真的能从零开始处理存量问题。Python仓库我用的是ruffmypy的组合。ruff负责格式和一部分可静态判断的lint规则mypy负责类型检查。这里要注意的是mypy在接入初期往往会因为存量代码类型标注不全而出现大量错误。我的经验是先开一个宽松阶段的配置比如只检查main包、忽略测试目录跑通之后再把严格开关一点点打开。一次想全量铺开通常以失败告终。前端方面ESLint其实已经是标配了但那套经典的airbnb规则对所有新项目并不是完全适用。我的建议是规则集的根尽量基于eslint:recommended或typescript-eslint/recommended再根据团队自己的编码习惯加上少量的自定义规则不要直接上大而全的规则集否则会有大量格式类争议耗尽大家的耐心。3.3 自动化检查与人工审查的衔接Reviewdog的接入过程在所有linter和静态分析器都跑出结果之后下一步就是把结果回写到MR。Reviewdog在这个环节的价值非常大不过它有两个细节容易踩坑。第一个坑是使用方式。Reviewdog有两种运行模式本地上手模式和CI模式。在CI里一定要使用-ci参数表示它运行在CI环境中否则它会假设自己在本地交互式终端里行为会不同。第二要指定reporter参数为gitlab-mr-discussion这样评论才会落到MR的Discussion里。如果是GitHub就把reporter换成github-pr-review。一个典型的Job配置长这样Go仓库go-lint: stage: test image: golangci/golangci-lint:latest script: - golangci-lint run --out-format line-number | reviewdog -fgolangci-lint -namegolangci-lint -reportergitlab-mr-discussion -levelwarning only: - merge_requests这里我把level设成warning意思是这些评论不会自动阻塞合并它们是给人类审查者参考的。真正要阻塞合并的条件我在第4章会细说。第二个坑是Reviewdog的token权限。很多团队图省事直接用一个读写了所有项目的Maintainer Token这会带来安全隐患。建议为Reviewdog单独创建一个Project Access Token权限只给该仓库的api权限为了发评论并且设为只读的Report角色。这样就算Token泄露影响范围也就是这个仓库的评论能力不至于影响到别的项目。3.4 把每一次审查结论存下来评论数据落库Reviewdog把评论发到MR上面了但如果我们不从GitLab API把评论拉出来做结构化存储那这些结论在项目结束后就散落在各个MR里没法检索。我是这样做的在Pipeline的最后一步加一个agent任务用GitLab API把当前MR的所有Discussion和Note拉下来过滤出reviewdog自动生成的和审查者手动标注的然后转存到一个PostgreSQL表里。表结构大概是这样字段说明mr_iidMR编号file_path文件路径line行号bot是否机器人产生的评论content评论内容severity严重级别created_at评论时间resolved是否已解决这一步看似多余但它的价值非常大。三个月之后你想统计我们最近有多少个PR是因为并发问题被拒的或者哪个模块的代码审查问题最密集直接从这个表里查就行了不需要把几百个MR从头翻一遍。我这个表是用Python脚本在Pipeline里写的也可以你自己写一个简单的Webhook接收器GitLab端设置既有的Webhook推送成JSON格式接收器存库。哪种方式都能用关键是把闭环建起来。4. 上线后必须处理的权限与门禁问题4.1 权限模型谁有权利批准什么工具链跑起来之后最容易被忽视却又最致命的是权限模型的设定。首先是谁可以批准MR的问题。我见过很多团队用默认设置仓库里所有人都可以approve结果就是你帮我approve一下变成了一种人情交换。我的做法是在GitLab的项目设置中把Approval Rules设为至少两位指定成员其中必须包含具备相应模块所有权经验的人。如果两人有位就把规则设为至少一人来自后端维护者组、一人来自QA负责人组。这样可以有效避免自己提交自己approve的情况。其次是提交者本人是否可审批。GitLab有一个设置叫Prevent author approval这个一定得打开。另外还要打开Prevent reviewers from approving their own review——这条很隐蔽但非常重要因为在某些情况下一个模块的Owner是提交者和审批者几乎指的是同一个人如果没有这个限制容易产生自己演独角戏的审查。我整理了一个权限清单你可以直接参照项目Maintainer权限只给技术负责人和CI管理机器人。Developer权限给普通开发允许创建分支和提交MR。禁止Author Approve和Self-Review。MR合并权限仅限Maintainer角色普通开发者即使有Approval也不能自己合并。设置MR过期未审查自动提醒72小时未处理就相关审查者。4.2 合并门禁策略哪些情况必须卡住在这个方案里合并门禁我设置了这么几个等级按严重程度从高到低排第一级是阻塞型问题。比如依赖中存在高危安全漏洞、引入了明确的数据竞争模式、覆盖率相比目标分支下降了超过5个百分点、mypy或TypeScript的强类型检查出现了error级别问题。这些条件触发任何一个Merge Request的合并按钮就应该是灰的——要人工排除以后才能走。第二级是建议型问题。比如代码里出现了重复片段、函数过长、某个组件的复杂度超标。这些由reviewdog发warning级别的评论但是不阻塞合并。这个层级是正确的切入点如果把所有lint问题都设为阻塞一方面会导致团队去规避规则另一方面会制造大量修lint的commit没有实际价值。第三级是模板要求。MR描述必须包含改动背景、测试计划、影响范围、自测截图或日志。如果没有填写机器人会在MR里留言提醒但不会硬阻塞。这种软约束比硬阻塞更容易让团队接受因为它在教大家什么样的MR值得被审查。这里要专门强调一下Coverage门禁。覆盖率变化应该基于全量项目当前覆盖率来比较而不是新增代码的覆盖率。因为后者的统计方式很容易被测试代码刷上去并不能真实反映带风险的改动是否被覆盖到。当然这个也不是绝对的但小团队我更建议看全量变化噪音会小很多。4.3 误报与噪音的治理让工具学会闭嘴自动化工具的灵敏度往往是个双刃剑。跑起来之后开发者最反感的不是有问题而是一顿操作猛如虎一看全是误报。我踩过一个非常典型的坑有一段时间ESLint的no-unused-vars规则对TS文件里的interface某一项会偶尔误报结果大量PR都带着一个unused variable的评论开发者点过去一看又不是。这种评论持续了大概两周团队里几个前端开发直接把reviewdog的评论当垃圾信息忽略掉了。后来我专门给这个规则加了一条exceptions配置把误报率降到接近零大家对工具的信任才慢慢恢复过来。这个案例给我的教训是在自动化工具刚上线的第一两个月宁可少检查一些规则也要保证每一条弹到MR讨论里面的评论都是有价值的。一旦审查工具被团队成员定性为噪音来源后续想要再唤起他们的信任就很难了。所以我还做了一件事利用上一章提到的评论落库表每月跑一次统计看看reviewdog发的评论里有多少被开发者点掉了有多少实际触发了代码修改。按每条评论的后续commit内是否改动同一位置来近似判断这条评论是否被有效处理。对有效处理率极低的规则直接禁掉或改成只记录不提醒。这个指标我简单实现了样本不多但很有代表性。5. 让团队成员从被迫审查到主动审查5.1 审查模版的心理学设计降低第一个评论的门槛工具链都搭好了接下来真正难的是人的问题。我发现新鲜事物倒入后最大的阻力不是工具难用而是不知道怎么开始。很多开发者的第一个PR被机器人和规则一堆评论淹没之后整个人是懵的——不是不愿意改而是不知道从哪一条开始改。解决办法其实很朴素给MR设置一个审查填空模板。不是那种高大上的文档而是直接在MR描述区写清楚几条引导性指令。例如背景这个改动想解决什么问题如果是Bug请附上复现步骤。影响面这次改动会影响哪些模块是否可能影响老数据测试计划你跑了哪些测试哪些没有覆盖自检清单是否处理了错误路径是否有日志帮助排查问题一句话描述示范Review的范围。这个模板的作用是把审查者的注意力引导到应该关注什么而不是让他在几百行的diff里自己找重点。如果一个MR本身就写不清楚这些那说明它还没有准备好被审查或者说它需要再拆分。5.2 数据驱动把审查指标放到共享看板上前面提到我存了所有审查结论到PostgreSQL那个表格的价值到这里就真正发挥出来了。我每个月会从库里跑出来一张审查健康度看板放在团队的共享大屏上平均首个评论响应时间从MR创建到第一个reviewdot评论的时间。目标4小时。平均合并等待时间从MR创建到合并的时间。目标24小时。自动审查阻塞率被自动化工具阻塞的MR占比。人工评论密度每个MR平均有效人工评论数。目标在5~15条之间。有效处理率评论被开发者后续commit呼应处理的比例。这些指标本身不是考核工具而是帮团队找到流程的瓶颈。比如如果人工评论密度长期为零说明大家的Review只是走个形式如果评论密度极高那可能是MR拆得太大了。透明化的指标能让问题暴露出来多数人看到数据的变化就会自动调整自己的Review习惯比任何制度的约束都管用。5.3 轻量化评审节奏把审查嵌进习惯而不是做成仪式最后一个想分享的经验是评审节奏的轻量化。我们在团队里约定了一个非常轻的节奏不设立独立的Review会议也不要求MR在特定时间点集中评审而是每天上午十一点前把待审MR列表统一丢到IM群里对应负责人下午四点半之前需要给出结论或明确说明预计明天几点能审完。下午四点半这个时间点我特别强调它不是硬性的deadline而是创设了一个默认约定让每个人心里有数如果今天不处理它就要主动告诉对方明天什么时候能处理。这比默不作声地拖到周五要好得多。这背后在用的其实是行为经济学里面的默认选项效应——当你不设定一个明确的预期时间尽快审这个模糊概念永远不是最优时间。回顾这大半年open-code-review的运行历程我个人最大的感受是这套方案并没有发明任何新技术——GitLab、Reviewdog、各种linter都是现成的——它更多的是把该自动化的事情和该人来判断的事情彻底分了层然后用统一的反馈和沉淀机制把它们衔接起来。如果你也想在团队里搭建类似的代码审查体系我的建议是从最小的闭环开始先给一个主力语言仓库配上增量式lint检查加Reviewdog回写MR的pipeline让团队拿到第一份自动审查报告再根据这一个月里出现的高频问题决定第二步做什么。工具不在多能让团队在合代码之前愿意多点开两次MR看评论就已经赢了一半了。
阅读完成 · 觉得有帮助?