搞代码审查这么多年我一直觉得这活儿本身才是团队里最需要被“审查”的一环。每次提到 code review老程序员想到的是又一轮口水战新程序员想到的是提心吊胆的“批斗会”管理层看到的是进度表上永远悬着的那根刺。直到我动手把日常做代码审查的流程、规则、脚本和检查清单沉淀成一个开源项目命名为 open-code-review才发现这事儿完全能变得顺畅、客观、还能自动跑起来。这篇文章就来聊聊我对这个项目的完整拆解从设计思路、核心功能、接入方式到实战中踩过的坑。不管你是想给团队引入一套更规范的 code review 流程还是打算自己写一个代码审查工具又或者只是想看看别人是怎么把 GitHub/GitLab 上的改动自动挑出毛病的这篇内容都能给你提供直接可复用的参考。尤其是那些“文档上不写、只有跑过才知道”的细节我都会毫无保留地交代出来。1. 这个项目到底解决什么问题1.1 先说痛点代码审查为什么让人头疼代码审查的本意是通过“人眼复核”来发现设计缺陷、逻辑漏洞、风格问题但实际操作中它常年被三个问题折磨。第一个问题是“看不全”。一次合并请求里动辄几十个文件、上千行代码评审人哪怕认真看了也容易漏掉几个边界条件或者只看 diff 里改到的地方没注意到那条改动对附近旧代码的连锁影响。人不是机器注意力有限这是生理限制不是态度问题。第二个问题是“标准不统一”。同一个团队里有人强制要求所有变量都要用 final 修饰有人觉得注释里不能有中英文混排有人只关心异常处理是否到位。审查意见一旦建立在个人审美上就容易变成“谁的嗓门大听谁的”技术讨论慢慢就变味了。第三个问题是“反馈太慢”。代码写完后等 review一等就是半天一天等来的意见还可能只是“命名改成 xxx”“这里加个空格”这种吹毛求疵的写法问题。等代码真正合并需求早就拖慢了项目节奏就这么一点点烂掉。这三点叠加起来让很多团队最后干脆放弃了正式评审改成“写完了自己看着没问题就推上去”。但代码质量和团队协作能力恰恰是在持续的 review 中磨练出来的。1.2 open-code-review 的定位与开发目标当初做 open-code-review 这个项目我给自己定了个非常朴素的调子把代码审查变成一个“机器能做就不要人工做、人工只做机器做不了的事”的流程。它不是一个替代真人评审的 AI 工具而是一个把“机械性检查”全部自动化的审查辅助框架。具体到能力上我希望它达到这几个目标。第一在代码提交到评审人手里之前先自动跑完一轮静态检查把风格问题、低级错误、明显的安全隐患提前过滤掉。评审人拿到手的应该是清理干净之后的代码而不是满屏红线的草稿。第二检查规则必须高度可配置、可扩展。不同语言、不同团队风格差异巨大所以规则集不能写死在代码里每个团队都可以定制自己的规范。这个非常重要否则项目落地到别的团队不出三天就会因为“规则不贴合实际”被弃用。第三审查意见必须有上下文、有依据。直接说“这里有问题”没有意义好的工具应该告诉提交者哪一行、属于哪个规则、为什么这么判、建议怎么改。这样开发者也服气也知道怎么调整。现在回看这个项目——其实我更多是在做流程梳理和规则工程化把散落在各个团队成员脑子里的审查经验变成一个可以版本化管理、自动执行、持续演进的体系。核心思路就一句话审查标准应该是可执行、可追溯的代码资产而不该是隐含在几个人脑子的私人记忆。2. 核心架构与设计思路2.1 整体架构入口、引擎、出口三层open-code-review 在代码结构上分成三层分别是入口层、引擎层和出口层。设计上参照了流水线的概念每一层各管一摊事层与层之间通过标准数据结构解耦。入口层负责获取待审查内容。最常见的是从 Git 服务器拉取合并请求/拉取请求的 diff 数据也支持直接指定本地 Git 仓库中的 commit 范围比如从git diff HEAD~1 HEAD里取变更内容。设计上尽量做了抽象只要能拿到标准的 diff/patch 格式数据就能接入不强制绑定某一家 Git 托管平台。引擎层是核心负责对 diff 进行解析、对代码执行多规则并行扫描、汇总结果、输出报告。引擎的内部模块包括diff 解析器负责把补丁格式的变更拆成“修改前的代码段修改后的代码段”文件类型识别器根据扩展名和文件内容判定语言类型然后匹配合适的语言规则包规则执行器接收文件级别或者代码行级别的输入依次执行命中规则结果统一进入结果聚合器。出口层负责把结果输出成可读报告支持终端文本、HTML 报告、JSON 数据三种格式。JSON 格式可以直接对接 CI 系统比如给成熟流水线发通知或者通过 API 在合并请求上直接进行评论HTML 报告适合在本地看效果每个检查项的命中位置都做了锚点跳转终端文本则适合在开发机本地快速验证。设计这层结构时有一个关键取舍为什么不把“在 MR 上自动评论”做到入口层因为我希望工具的引擎部分是完全本地化、可离线运行的。如果把工具本身变成一个云端评论机器人虽然使用更方便但企业内网环境就很难部署了同时也引入了代码泄露到第三方的风险。源码得留在自己手里跑这是很多研发团队对代码审查工具的底线要求。2.2 以“变更”为核心不做全量扫描这里说一个我认为 open-code-review 最关键的架构原则审查目标只针对本次变更的 diff而不是对整个代码库做全量扫描。常规静态分析工具比如 SonarQube 那类大多是做全量扫描的虽然能发现存量问题但存在两个明显弊端一是每次扫描耗时太长二是有历史技术债存在时新引入的问题容易被淹没在海量告警中。而代码评审关心的重点是“这次改动引入了什么新问题”而不是“十年前就存在的历史遗留问题”。所以在 open-code-review 中引擎先拿到 diff再识别“新增了哪些行、修改了哪些行、删掉了哪些行”最后只对被“新增和修改”的行执行规则检查。这样带来的好处非常直接单次审查耗时控制在秒级可以支撑 CI 同步调用用户无需异步等待告警数量少而精准命中位置全部都在改动行内历史问题不会混进来搅局开发者的注意力聚焦在“本次变更是好是坏”上。实现上diff 的解析并不复杂Git 的 diff 本质上是带行号的块hunk每个 hunk 里有new_line_number范围我们把它展开成一个“新增行 → 文件第几行”的映射表规则报告命中位置的时候查一下这张表就能确认是否属于新增行。如果命中位置不在表里说明问题出在未改动区域默认降级为警告而非错误避免误伤。3. 快速接入从零到第一个真实审查报告3.1 环境要求与安装方式这个工具用 Python 3.9 开发依赖 Git 命令行工具。选 Python 不是因为性能最优而是因为做规则引擎这种偏重生态和灵活性的场景里Python 的规则表达能力最强不会有像 Java 那样的编译和反向代理成本也方便团队自己加规则不用重编译整个项目。安装建议用 Pip 直接装源码方式也支持。# 建议在虚拟环境中安装 python -m venv review-venv source review-venv/bin/activate pip install open-code-review装好之后验证版本号ocr --version我这个版本里命令统一使用ocr作为入口名总比每次敲全名方便。安装完成后在项目根目录下初始化和生成配置文件ocr init这条命令会在当前目录生成一个.ocrconfig.yaml的配置文件和一个review_rules/规则目录。配置文件集中管理全局开关规则目录则按照语言拆分子目录后面讲配置的时候会详细展开。3.2 配置项与最小可用运行配置格式我用的是 YAML因为它有注释支持比纯 JSON 更适合给团队传阅。刚初始化出来的配置内容大概是这样的# open-code-review 主配置 project_name: example-service language_presets: - python - javascript rules_path: ./review_rules report_format: terminal fail_on_error: false diff: provider: local_git base_revision: HEAD~1 compare_revision: HEAD这里有几个关键配置说说我的取舍。“language_presets”是启用哪些语言规则包优先准确控制扫描范围避免无关文件引擎全跑一遍浪费时间。“fail_on_error”控制审查发现错误的时候是什么行为如果设成 true在 CI 里一旦发现问题流水线直接挂掉设成 false则只提醒让流水线继续跑。个人建议在推行初期把fail_on_error设成 false先让团队适应工具的节奏等规则打磨稳定了再切到强拦截模式。这步不要性急强制拦截一旦伤到业务责任人往往不再用这个工具了。“diff”配置块里provider 设成local_git时引擎直接对比本地 Git 仓库的两个版本如果想用在 CI 环境可以换成env模式从环境变量里读仓库地址和 MR 编号省掉手动指定版本号。弄好配置之后本地测一下能不能跑通ocr scan这条命令执行后引擎会比较HEAD~1和HEAD两个版本的差异对新变更执行规则集然后把报告打印到终端。第一次跑就成功的概率其实不高大概率会遇到 Git 仓库路径找不到或者 Python 版本不兼容的问题遇到别慌先看具体的报错栈后面的排查章节会给出我们踩过的那些坑。4. 规则体系与自定义规则编写4.1 内置规则包分类与实现对比例子open-code-review 内置的规则我做成了按严重级别划分的等级体系错误、警告、提示。错误级别针对会导致业务功能出错或安全漏洞的问题警告级别针对可能导致潜在风险或明显与团队约定不符的问题提示级别更多是优化建议和代码风格层面的提示。拿一个真实的规则实现看下效果。比如“未使用变量检查”常见于改动过程中删掉了对某个变量的使用但声明还留着。如果直接扫描整份文件命中位置可能很多但我们只关心 diff 里出现的行所以扫描范围大大缩小也更有实际意义。实现原理上分成三步先是提取 diff 中新增的行然后做语法分析解析变量声明与引用最后比对引用次数声明过但引用次数为零的就报一个提示级别的警告。这个检测用抽象语法树来完成。把这类规则写进引擎的好处是它能准确区分“局部变量未被使用”和“模块级导出变量”后者在 Python 中可能是被外部导入的报未使用反而会产生污染的噪音。内置规则包里还有一些值得一说的“函数行长检查”属于结构风格类正则匹配函数体代码块统计行数超过约定上限的给出提示“危险的动态执行函数调用”属于安全类比如检测到对eval、exec的调用时会触发警告同时会认真看一下被执行的字符串是否来自外部输入“调试残留检查”针对debugger、print本身不触发 IDE 断点但和团队约定的有效实践结合起来能及时筛出那些忘记撤销的调试输出“异常捕获过宽”针对 Java/JavaScript/Python 的裸except Exception或catch (e: Exception)建议缩小异常捕获范围防止真正的运行时错误被静默吞掉。规则的表现形式是代码文件形式打包的每条规则的核心是一个扫描函数输入文件行号和上下文输出规则结果独立性强也便于扩展。4.2 手写一个自定义规则有多简单自定义规则是我觉得这个项目相比其他工具体系的一个优势点——它带来了二次协作的安全感。工具的默认规则达不到团队当前的技术要求时团队可以自己补上自主可控。我在设计规则接口时特意简化了约束规则的实现主体就是一个包含check方法的BaseRule子类check的输入是文件上下文对象输出是一个Issue对象列表。先看一个完整示例拿 Python 代码中的 TODO 注释治理来说from ocr.rules import BaseRule, Issue, Severity class AvoidTodosInNewCodeRule(BaseRule): name avoid_todos_in_new_code description 新提交的代码中不允许出现 TODO 注释 severity Severity.WARNING language [python, javascript] def check(self, context): issues [] for line in context.new_lines: # 只遍历新增行 stripped line.strip().lower() if stripped.startswith(# todo) or stripped.startswith(// todo): issues.append(Issue( line_nocontext.get_line_number(line), message新代码中出现了 TODO请处理具体事项后再提交或关联任务编号, suggestion删除 TODO或在 TODO 后附带 issue 编号 )) return issues刚入手时别急着写复杂规则先从一个具体的、团队真实发生过的问题开始。只要确认这个规则足够“敏锐”不会频繁误伤就可以放入主规则集并配合版本管理来持续演进规则。规则文件放好后需要更新主配置启用它custom_rules: - ./review_rules/avoid_todos.py跑完一次扫描如果规则命中报告里会列出文件名、行号、严重级别、命中详情和修改建议。在 code review 的场景里这种完整信息能明显降低开发者的辩驳率——有凭有据而不是一句干巴巴的“这里不专业”。4.3 规则编写时的三个边界控制自定义规则踩过的坑比较多分享三个重点边界控制的经验。优先级最高的一个边界永远只扫描增量行不要顺手扫描整个文件。我一开始写的部分规则也会不自觉地去扫整个文件一旦这么做报告必然会被存量代码自身的问题淹没开发者看三行就直接不想继续了。所以在规则接口设计上我直接提供了new_lines属性只拿新增行给自定义规则这种“结构上强制”的做法比任何口头约定都有效。第二个边界是规则的执行时间耗尽问题。某些安全规则需要正则加词法分析如果表达式写得不好可能出现灾难性回溯。现在我在引擎里为每条规则设置了硬超时通常单文件不超过 500 毫秒超时自动终止并记录日志。自定义规则其实也该有超时意识。第三个边界是规则命中后允许“被豁免”。有些规则命中结果确实存在但当前引入问题是有意识的权衡比如“允许在文档字符串中使用危险函数”这种情况。在引擎实现上是支持静态阈值放宽的未来也会加入按文件名豁免的模式但现阶段我更建议用规则本身的降级处理来做避免豁免清单成为卫生死角。5. 与 CI/CD 集成让机器先审一遍5.1 基于 GitHub Actions 的接入示例本地能跑通之后接下来最关键的一件事就是把 open-code-review 接进 CI让每次推送都会自动触发一次机器审查。对于托管在 GitHub 上的仓库我用 GitHub Actions 作为示例来讲核心逻辑并不复杂。需要在仓库根目录新建一个 workflow 文件.github/workflows/code-review.yml写入下面内容name: open-code-review on: pull_request: types: [opened, synchronize] push: branches: [main, develop] jobs: review: runs-on: ubuntu-latest steps: - name: 检出代码 uses: actions/checkoutv4 with: fetch-depth: 0 # 关键需要拿完整历史 - name: 安装 open-code-review run: | pip install open-code-review - name: 执行代码审查 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} DIFF_MODE: github_pr REPO_NAME: ${{ github.repository }} PR_NUMBER: ${{ github.event.pull_request.number }} run: | ocr scan --diff-mode $DIFF_MODE这个 workflow 里有一个非常容易踩的坑如果忽略深层抓取Actions 默认只克隆某个分支的浅表历史这样compare_revision时的基版本可能不存在diff 会拿到一团糟的数据。所以必须用fetch-depth: 0确保能拿到该 PR 在合并前的所有提交记录引擎才能只对准确的字段范围做增量审查——这里绝对是实战经验上的重要提醒。仓库的自动评论接口我在引擎里也做了预置支持配置了 GITHUB_TOKEN 后引擎检测到github_pr模式会自动调用 GitHub 的 review 评论 API把命中项以逐个文件评论的形式挂在代码行上。这一步非常顾全开发者体验因为“改完刷新一下页面看到代码行旁边有自己项目的审查建议”这件事对团队接受度是大有加分的。5.2 基于 GitLab CI 的接入与差异点国内团队用 GitLab 的比例相当高所以 GitLab CI 的接入方案我在项目里也做了同样级别的支持。rules的判定在 GitLab 里需要把 MR 的元信息带进流水线所以配置上会略有不同image: python:3.11-slim stages: - review code-review: stage: review script: - pip install open-code-review - export DIFF_MODEgitlab_mr - export MR_ID$CI_MERGE_REQUEST_IID - export GITLAB_TOKEN$GITLAB_REVIEW_TOKEN - ocr scan --diff-mode $DIFF_MODE only: - merge_requests用 GitLab 时有一个重要差异需要注意它的合并请求流水线不允许在默认的短分支上直接访问目标分支初始情况。因此必须指定流水线运行时“同时获取目标分支代码”打开项目的 CI/CD 配置页把“合并请求流水线”和“目标分支获取”都设为开启否则引擎在解析 diff 时会找不到 base tree。细节到位之后效果就非常稳了一个 MR 推上来几分钟内机器审查结果就挂在代码行上评审人再来审的时候看到的已经是过滤掉机械问题后的内容。此时的人工评审才有精力真正投入到“全局设计是否合理”这件事上这正是代码审查环节里最有价值的部分应当被有效保护。6. 实战演示一次 Merge Request 的完整审查记录6.1 运行效果与报告解读先看一个精简版的场景。假设某次 PR 里新增了一段 Python 代码用于处理用户上传的头像文件。本次提交涉及一个profile.py文件改动内容大概这样def save_avatar(user, file_path): img open(file_path, rb) data file.read() user.avatar_data data user.save() return ok跑一遍ocr scan之后终端输出报告大概是[open-code-review] 项目 example-service 审查完成 扫描文件: 12 | 新增代码行: 346 | 命中问题: 3 审查报告 文件: api/profile.py 行 87 [错误] 未关闭文件句柄: 使用 open() 后未触发 close() 建议: 改用 with 语句管理资源上下文 行 90 [警告] 潜在未定义变量: 使用未定义的局部变量 file 建议: 变量应为 img 或显式定义 file 行 82 [提示] 魔法字符串: ok 作为状态返回不利于维护 建议: 提取到常量或枚举类型 共 3 个问题1 错误, 1 警告, 1 提示按严重程度降序展示这个流程演示了工具核心价值所在。第一处错误可以靠人工仔细审出来但问题是它不适合作为评审日常放在口头约定里反复提醒第二处本质上是笔误img和file用反了代码能骗过“看着像没问题”的目测却绝骗不过语法分析第三处提示让老项目里散落的魔法字符串坏味道更早暴露。有了机器审查提交者自己提交前先跑一遍本地扫描这类错误就不会流到评审人那里。而评审人在这个基础上只需要做结构性的判断比如头像文件过大时该不该限制大小、要不要校验文件类型、是否需要异步处理图片缩放而不是浪费时间指出“你开了一个文件忘了关”。6.2 报告格式切换与解读配置终端报告适合开发机自验但 CI 场景里我更建议输出 JSON 格式方便后续脚本做透传ocr scan --format json review-report.json输出的 JSON 结构设计得比较平{ project: example-service, summary: {files_scanned: 12, issues_total: 3}, issues: [ { file: api/profile.py, line_no: 87, severity: error, rule: ensure-file-close, message: 未关闭文件句柄: 使用 open() 后未触发 close(), suggestion: 改用 with 语句管理资源上下文 } ] }summary快照提供给 CI 统计使用比如最近一周的告警趋势和规则命中频率这些数据对团队梳理技术债非常有参考价值。建议每个月把 JSON 存档用脚本汇总一次问题分布能比较客观地看到哪类问题最常出现倒逼规则库持续演进。7. 接入过程中的高频问题与处理经验7.1 拿到“扫描完成0个文件”时的排查思路这是新人刚接入时最高频报案ocr scan跑完提示“共 0 个文件”。通常不是工具坏了而是配置里对 diff 范围的理解出了问题。常见原因有三个。第一个原因是 Git 历史获取深度不足。在 CI 里拉取的提交历史只有一两层导致HEAD~1指到不存在的节点Git 静默地返回一个空 diff。排查方法是先跑git log --oneline -5确认父提交是否可用不可用就把fetch-depth设成0或者改成用 CI 统一环境变量传的“基分支名”。第二个原因是扫描范围只覆盖了未被忽略的文件。新加的规则没有开启language字段或者匹配条件没写上引擎无法识别文件类型就自动跳过了。建议用--verbose模式跑一次它会把每个识别到的文件和处理结果都打出来清楚交代了哪些进、哪些没进。第三个原因是规则集目录里确实没有任何可用的规则文件。如果从零开始初始化项目没在配置里写包依赖也没放自定义规则引擎就无规则可跑了。工具初始化时会同时生成语言预置包按初始化提示确认一下即可。7.2 误报太多引起的信任危机误报是代码审查工具的隐形杀手。一次误报会被开发者记很久后续就算修了也会被认为“这工具不靠谱”。要维护团队信任降低误报率是项目的核心运营目标之一。误报高怎么办先从最粗暴的自动降级机制做起。当系统对某个判断不是特别有把握时与其直接判错不如统一降为警告或提示级别如果开发环境本身是在保留历史技术栈的前提下改造的建议先只用“提示”级别跑两周收集真实命中率样本后再决定要不要提升某个规则的自动判级。第二招是优先启用“能明确执行”的规则。比如“未使用变量”在语法树判断里几乎没有摇摆空间可以直接设为错误而类似“函数规模过大”这类基于行数的启发式评定适合稳定在警告级别给人留出解释的空间。基本原则就是定级依赖结构性判断的规则少依赖风格或者规模的启发式判定。第三招是给命中项提供“可点击、可解释”的支撑。这一点在报告格式中特意做了“建议”字段的规范化每条命中必须给出具体改进建议而不是干巴巴的规则编号。开发者看到的不再是“你违反了 xxx 规则”而是“应该用 with 上下文管理资源”沟通成本降下来抵触情绪自然减弱。7.3 CI 流水线超时与性能调优建议仓库稍微大一点之后扫描时间的把控会成为一个体验瓶颈。我现在的大型仓库跑一次全流程约 20 到 40 秒在可接受范围内但对超大仓库来说这个时间会进一步拉长。优化方略可以根据实际情况选择先把语言预置包配置到最小集合默认不启用所有语言避免加载永远不会用到的规则对单文件执行做超时阈值控制超会话自动跳过并写日志至少保证个别异常规则不会拖垮整条流水线开启规则级缓存引擎会按“文件名最后提交哈希”缓存结果未变更文件直接复用上次结论可显著省时在 CI 上把扫描拆成和编译并行执行不要在某些阶段排队等待资源时间充裕是有价值的。这些优化说白了还是开头说的那个原则代码审查这事本身不该成为开发节奏的负担。如果在跑完这轮审查之后流水线时间涨了一倍以上那工具和流程之间是需要重新做一次时间权衡的。8. 从工具到机制推进团队代码审查文化的三个建议技术活做完之后最难的其实还是“人”的层面。open-code-review 跑起来了规则也会命中但怎么让团队真正接受这套审查文化、持续用下去我复盘下来有三条建议是值得特意写出来的。建议第一条机器挑出的问题千万别跟人收割“绩效”挂钩。工具的作用是提醒大家共同规范不是给谁贴标签。我们早期把“新增严重问题数”列入个人指标后团队立刻开始用武力式措施绕过规则比如把两行逻辑写到一行、将临时变量改成名单上绕路判断这样的时间成本远大于工具带来的收益。后来改成团队维度看趋势每周发一份进行中的问题趋势图不点名只提醒接受程度反而好了很多。建议第二条先解决“让提交者在提交前自己跑一遍工具”而不是上来就强制 CI 阻断。本地养成自检习惯之后CI 端的阻断自然变得不那么扎眼。为此我在工具的 README 里特意写了用户快速开始段落顺手提供了pre-commit钩子的安装方式几分钟配好每次git commit自动触发本地扫描。建议第三条规则库要有人长期维护。项目一旦接入真实团队规则也必须跟着团队的技术栈变化持续演进。我比较推荐每两周留出半天时间拉上团队里写代码风格最较真的那个同学把近期人工评审里出现的共性问题固化成新的规则。这样一来每次规则的增加都对应着一次实际评审经验而不是凭空想出来的花架子规则。从工具到机制最底层的变化是审查标准从“老师和学生”的对抗关系变成了“自动化的公共意志”。大家看同一套规则、用同一套工具省去了解释成本真正的设计讨论反而多起来了。这也是我做 open-code-review 这么久以来最欣慰的地方——它不是在制造新的流程负担而是在把团队的注意力拉回到代码本身上面。如果你也打算给自己的团队引入类似的实践我的建议是不要贪多求全先从小处着手。挑一种语言、配置十个以内最重要的规则、跑一个月看完数据再决定要不要扩大范围。工具是死的规则是活的真正重要的是把审查当作一种需要持续打磨的团队能力来对待。
