ARTICLE · INTELLIGENCE

战地情报 · 详情页

来自尧图项目组的一线实战观察与深度解析

开放式代码评审实践:从私聊到公开协作的流程改造

开放式代码评审实践:从私聊到公开协作的流程改造 我是在一次评审卡壳三天的早上动了要把代码评审“敞开”做的念头。那次线上 ticket 已经 Ready 两天唯一有权限合入的同事在异地出差群里 了三次没人回。问题的根源不在人懒而在评审链路被设计成了一个私密单点作者找一位评审评审说 OK 就合入。后来我牵头在组里推了一个叫 open-code-review 的项目核心理念就一条——把代码评审从“两个人之间的对话”变成“全组可见、异步参与、可追溯的公共事务”。这篇文章记录了这个项目从方案设计、工具选型到落地推行的大部分细节如果你也在苦恼评审卡顿、质量靠自觉、新人找不到人评审这类问题可以参考这里的思路。1. 事情是怎么变麻烦的评审卡顿、黑盒决策与数据荒任何流程改造前提都是清楚地知道自己在痛什么。我当时把团队里关于代码评审的抱怨列了一遍发现看起来五花八门其实里面就三类问题卡在节点上、黑盒里决策、拍脑袋改进。1.1 三个典型症状越拖越疼卡在节点上是最容易感知的一个 MR 推上去之后大家默认“已经有人在看了”但实际等多久完全凭运气。评审人如果当天开会多、任务重MR 就躺在那儿一动不动。新人更明显入职前两周想提代码连该找谁评审都不知道只能挨个私聊求人。黑盒决策是更隐蔽的问题。早期我们按“一对一评审”走作者和唯一评审在私聊或评论里来回讨论最后达成一致、合入代码。看起来流程没毛病但组里其他人完全不知道为什么要这样改等出了问题想追上下文只能翻聊天记录。更难受的是如果那位评审当时拍脑袋放行了后面的人会觉得“这个是某某人同意的”没人愿意去推翻。数据荒则让所有改进都失去依据。那时候如果有人问我“咱们评审到底多快、哪个模块问题最多”我答不上来。没有统计口径没有沉淀所谓提高评审质量只能靠喊口号。项目名 open-code-review 里的 open其实就是冲着这三件事去的打开卡住的节点、打开讨论的黑盒、打开可度量的数据。1.2 藏在症状背后的共性评审被当成了“私聊”分析到最后我发现所有问题的根因是同一个代码评审在流程设计上就是一种“私聊”模型。作者拉一个人两个人对完话事情就结束了。群里默认的“整个团队都参与评审”只是美好愿望没有机制支撑。私聊模型有三个连锁后果。第一可见性极差别人看不到过程只能看到结果第二参与面被压到最小没被拉到的人永远不会主动去看因为系统没有给任何入口第三责任容易蒸发一旦出了问题很难回溯出当时究竟谁拍板、依据是什么。直到我开始在 open-code-review 项目里做设计才明确意识到要么接受代码评审天然是私聊要么就得在工具和流程层面把“公开”变成默认行为。1.3 设计原则怎么定下来所以 open-code-review 的第一个产出其实不是代码是三句话默认透明所有评审讨论默认全组可见不作私下沟通。异步优先不要求评审人立刻实时响应但要给出明确响应时限。小步提交MR 小到评审人能在 15 分钟内过完避免超大 diff 让评审失效。这三句话看起来简单落地时很折腾。例如“默认透明”意味着连“作者提前和熟人打招呼”这个习惯都得改新人刚开始很不适应总觉得自己写得不成熟公开贴上墙有点丢人。但后来发现正是这种不适感倒逼大家把代码整理得更完整、提交说明写得更好。另外一个很实际的好处是评审讨论公开之后团队里其他人哪怕不发言也能通过只看讨论学到东西——这种隐性的传帮带是私聊模型永远做不到的。2. 工具选型Gerrit、GitLab MR 和自研 Bot我最后选了哪条路决定“敞开评审”之后下一个绕不开的问题是工具。从零写一套评审系统显然不现实市面现成方案里Gerrit 是“评审优先”的老牌选手GitHub/GitLab 的 Merge Request 流程则更偏协作。我和团队围着这两个方向吵了大半天后来还是决定在 MR 流程上做增量改造而不是引入一个全新的评审平台。2.1 直接照搬 Gerrit 为什么不划算先说 Gerrit。它在对代码评审极其严格的团队里很受欢迎尤其是那些需要逐行审、逐 commit 审的场景。Gerrit 的核心模型是“每一行代码都要经过严格 review 才能合入”Pull/Merge Request 里常见的批量讨论在 Gerrit 里并不占主导所有评论挂在具体 commit 上。这对严谨度要求极高的基础架构组是优点但对我们的业务团队来说是一次全流程切换。我们当时最现实的顾虑有三个。第一Gerrit 和 Git 工作流的结合要重学一遍老员工抵触情绪不小第二Gerrit 没有现成的 CI 集成而我们的流水线本来就围绕 MR 事件搭好了大半第三Gerrit 的权限模型细到令人发指配置成本高。对于一支已经开始用 MR 做日常协作的团队引入 Gerrit 等于为了修一个门锁把整扇门换了。所以“不划算”不是 Gerrit 不好而是它和我们团队的上下文不匹配。2.2 在成熟 MR 流程上做“增量改造”最后敲定的路线是保留已有的 GitLab MR 流程在此基础上做一套轻量评审工具也就是后来的 open-code-review。为什么选 GitLab 而不是 GitHub主要是私有化部署的灵活性GitLab 实例就架在我们内网Webhook、API、MR 模板、CODEOWNERS、分支保护这些能力都齐全开箱即用。GitHub 当然也能做但当时团队权限和账号体系已经和 GitLab 绑死迁移成本没必要。所谓“增量改造”其实给了自己一个边界平台已经具备的能力绝不重复造轮子。分支保护、权限角色、评论弹窗、MR 状态流转这些都直接用平台原生功能我们要补的主要是三个薄层——评审人自动分配、评审清单下发、评审质量统计。这三个薄层用一个 Bot 服务来接 Webhook 事件再把分析结果写回 MR 评论。2.3 评审 Bot 的职责边界工具做得越大越容易死。所以 open-code-review 里的 Bot 组件职责被我严格框死在三件事内。第一是接入 MR 的 open/update 事件根据改动文件自动算出一份“建议评审人”清单并 出来。第二是根据仓库根目录下的 .review-checklist.yml 找到当前 MR 需要勾选的检查项贴成一条固定评论。第三是把每天、每周的评审数据推送到内部看板写成评论或消息卡片。不做的事也很明确不写自己的代码托管不做冗长的自定义审批流不接管最终合并权限。因为评审工具一旦膨胀成一个大平台维护成本会反过来吞掉它带来的效率收益。我不想把 open-code-review 变成团队的负担所以从一开始就保持“薄”的定位平台能干的交给平台我们只补平台缺的那一块。3. 落地的三条水管提交规范、评审清单与自动分配架构定了真正开始配置的时候才发现核心流程设计里全是容易被忽略的细节。这一节我会把三个最影响体验的部分拆开讲提交信息约定、评审清单、评审人自动分配。这三件事做扎实开放式评审就等于铺好了水管后面水代码讨论自然会沿着管道流。3.1 提交信息和 MR 模板先让上下文完整在请别人看代码之前第一步是帮他用最少的时间搞懂这段代码为什么存在。我见过太多 MR标题就写“fix bug”“update code”点进去还得靠猜。这在开放式评审里是灾难因为围观群众没有耐心去考古。我们强制推行了一套提交信息约定格式是type(scope): subjectscope 写模块名subject 一句话概述问题正文里必须带对应 issue 链接和修改思路。IDE 和 Git 配合起来后人均额外耗时不超过 1 分钟回报却很划算评审人一眼就知道“这是什么改动、为什么改、影响面在哪”。MR 模板同样关键。我们在仓库里放了.gitlab/merge_request_templates/Default.md里面固定几个小节背景、改动点、测试情况、回归风险、以及评审人特别需要关注的地方。这个模板强制作者把评审人想先知道的信息提供完整避免评审过程中反复来回追问上下文。提示很多团队把提交规范当纪律来抓效果一般因为罚人不是目的。更好的办法是让规范替你省时间——模板产生后直接把验收标准写进去作者没填完反而过不了 CI这比任何口头要求都可靠。3.2 评审清单把“经验”变成“可勾选项”老手评审靠感觉新手评审靠运气。为了把“经验”沉淀为可执行项我们设计了仓库层面的评审清单文件.review-checklist.yml。Bot 会根据 MR 改动的文件路径把匹配的检查项自动渲染成 MR 评论。一个简化示例rules: - pattern: backend/**/*.py checklist: - 数据库查询是否命中索引 - 是否处理了事务边界 - 是否存在潜在的空指针风险 - pattern: **/controllers/** checklist: - 入参校验是否完整 - 错误信息是否可读 - 是否包含越权风险路径 - pattern: **/migrations/** checklist: - 是否包含新增索引 - 是否是纯增量、可回滚这套清单不是写一次就完事它应该跟着团队踩坑记录持续迭代。每次线上故障复盘发现了新问题就往对应路径下加一条每次评审中大家反复提起某种坏味道也沉淀成检查项。一个月下来清单会慢慢变成团队的“活文档”效果比让每个人都记住一堆最佳实践好得多。3.3 自动分配评审人让代码找对的人开放式评审要避免“全员评审、人人不管”的乌托邦所以还是得有一个“第一责任人”。但我们不是固定指定一个人而是用一条规则去动态算。我的实现思路是给每个潜在评审人打一个综合分该模块的历史提交贡献度谁的代码多谁就有义务持续 review 相关改动评审该模块的历史次数看得多更熟悉上下文当前周期内已分配的 MR 数量做负载均衡防止固定评审人被淹没Bot 每收到一个新 MR就按这三个维度从 CODEOWNERS 和 Git 历史里跑一遍得到前两名推荐评审人并 到 MR 里。核心逻辑是个很简单的加权公式实际效果主要靠权重调参score ownership_weight * gitlog_owned_lines review_experience_weight * historical_review_count * 0.6 - load_weight * current_assigned_count初衷很朴素让相关代码的长期维护者优先审让手头排期最轻松的人也优先参与别把评审集中到几个人身上。这套自动分配上线之后最明显的变化是新人的 MR 也能被分到合适的评审人不用再挨个私聊问“你能帮我看一下吗”。3.4 CI 门禁与人工评审的分工还有一个很容易踩的坑就是把自动化检查和人工评审当成一回事。很多人以为 CI 跑绿了就等于评审过了这是个非常危险的误解。CI 能验证的是“代码在机器上不炸”人工评审补的是“这段改动是否值得做、结构是否合理、未来的坑在哪”。我们在分支保护里设了两道门ERB 门禁和人工批准门禁。自动化的检查包括编译、单测、代码规范、覆盖率必须全部通过同时必须至少有一个满足权限的评审人点了 Approve 才能合入。两道门不互相替代而是在时间线上天然衔接机器先快速过滤低级问题人工再集中精力看高级问题。这么设计的另一个好处是评审人体验更好。给评审人的 MR 不用再收到一堆“少了个分号”这类机器就能抓的评论而是要回答真正有价值的问题。后来团队里对评审态度改观很大程度上就是靠这个细节——人只愿意做机器做不了的聪明事。4. 开放与控制怎么平衡权限模型和分支保护“开放式评审”听起来像一个全员随意 Approve、谁都能合入的无政府流程实际上恰恰相反。真正可持续的开放是在权限上有极其清晰的边界。把“可评论”“可赞成”“可合入”三个角色拆开是这个项目里我认为最有价值的设计判断。4.1 “可评论、可赞成、可合入”三个角色拆开我先定义了三类能力它们是层层包含的关系可评论任何人都可以进行讨论、提问、提出异议。这是开放的基础。可赞成有资格给这个 MR 点 Approve代表“我以个人名义背书这个改动”。可合入有权限执行合并动作通常只给仓库维护者。团队最开始的质疑是“所有人都能评论那评审还有权威性吗”答案依赖一个前提评论是建议赞成才是正式背书合入则是最终技术决策。开放式评审鼓励的是评论层面的充分参与但把关动作仍然收敛到熟悉模块的评审人手里。这套模型既解决了信息透明又保留了决策严肃性。我在别的团队见过另一个极端只要拉了人就算评审过点个 Approve 就瞬间合入。这在内部工具团队可能没问题但对核心业务代码来说太危险所以权限拆分必须用平台机制锁死不能靠自觉。4.2 CODEOWNERS 与分支保护的实际配置GitLab 的 CODEOWNERS 是我做权限收敛的主要抓手。拿仓库示例来说# 根目录全局 owner * platform-core/maintainers # 后端改动必须有后端负责人 /backend/ backend-owner # 数据库迁移文件必须由 DBA 组确认 /migrations/ db-maintainers每个 MR 只要触达对应目录系统就会自动要求相应组的 Approve。这与分配评审人是两套机制分配评审人是“建议谁来看”CODEOWNERS 是“强制谁点头”。两者配合既照顾效率又不牺牲关键路径的把关强度。分支保护方面我们把主干分支设置为禁止直接推送、必须通过 MR 合入、必须流水线通过、必须满足至少一个代码所有者批准。这些平台能力配置一次就好但每个人都应该明确知道保护的不是“谁有权”而是“改动怎么流进主干”这套规则。4.3 开放评审后容易失控的三个信号流程跑起来之后更要留意的不是不会用而是被用歪。我总结过三个失控信号团队里出现任意一个就要立刻回头调配置。第一个信号是 Approve 泛滥。如果一个 MR 经常攒五六个 Approve未必是大家真觉得好更可能是“人情 Approve”——来看一眼觉得差不多就点了没人愿意当恶人。第二个信号是评审人长时间单点。比如某个模块永远只有一个资深同事在批一旦他休假整个迭代就卡住。这种情况要考虑再培养一名模块副 Owner或调低该模块的强制 Approve 人数。第三个信号是评论质量下降比如越来越多的“1”“LGTM”“OK”式评论。这不是说不能简短批准而是如果所有评审都只有这种短评论大家大概率只是在走过场。出现这些信号时我会先看数据——哪个模块的评审周期异常长、谁是成批复阅机器——再决定是调 Bot 的分配权重还是找团队聊聊评审文化。工具只能制造可能性让机制不烂掉靠的是持续观察。5. 数据说话评审周期从 2.1 天降到 0.8 天以及中间踩的坑没有数据的流程改造等于闭眼开车。我们在 open-code-review 里定义了四个核心口径跑了两个季度之后团队评审周期从平均 2.1 天降到 0.8 天。数字很漂亮但过程里踩了好几个坑我觉得比数字本身更有参考价值。5.1 我先定义了四个可量化的口径数据要对比前提是所有人对口径有一致理解。我们固定用四个指标不贪多评审响应时间MR 从提出到第一位评审人发表评论的平均时长看“有没有人理”。评审周期MR 从提出到合入的平均时长看“多久能合入”。评审深度平均每个 MR 收到的非 “LGTM/OK” 类实质评论数看“是不是真审了”。变更规模每次 MR 平均增删行数看“提交是不是够小”。其中评审响应时间是最先改善的因为 Bot 自动化 让“有人理”这件事不再靠运气。而变更规模则是团队执行“小步提交”约束后的自然结果MR 变小了评审压力变小响应速度和深度自然都上去了。5.2 第一批推广就翻车机器人把人分错了怎么办工具第一次全量上线我觉得自己很懂结果第一周就被现场打脸。Bot 自动分配评审人的逻辑用了 Git 历史里谁改动多就分给谁的权重结果一段时间里大量 MR 都指向同一个老哥因为他以前贡献了最多的业务代码。问题出在我的负载均衡权重没调好。初期current_assigned_count的惩罚系数设得太小等于没起作用。而且单纯按提交行数统计会让“写了大量样板代码”的老员工被动成为所有模块评审的接盘侠。后来我们改了计算规则优先按 CODEOWNERS 所在组过滤再用同组内的 open MR 数做负载均衡同时把机器人自动分人结果从“确定评审人”降级成“建议候选”最终分配由作者确认或替换。这套“Bot 建议 人工确认”的互补模式上线后分错人的投诉基本消失。评审工具再智能也不能完全剥夺作者对自己 MR 挑选评审人的决定权这个度很重要。5.3 “已批准”不等于“真评审”应对空壳 Approve跟踪评审深度时发现一个沮丧的趋势MR 平均实质评论数一度低到每 MR 不到 1 条。说明审批是有了但不代表有人真的深入看。深入排查后我们发现大部分 Approve 来自机器人 出来的评审人他们只被动地打开 MR瞟一眼改动不大没细想就点了通过。我没有选择强制要求“必须写评论才能 Approve”因为那只会逼出更多没营养的“写句废话”流量。更有效的办法是引导评审人关注 15 分钟内能看完的小 MR同时把清单问题嵌入评审流程让 Bot 明确提示“这次 MR 涉及事务边界请重点检查”。一个原本不知道从何看起的评审人看到具体检查项之后至少有了提问的抓手。与其惩罚空壳 Approve不如把评审变成一件“知道该看什么”的事。5.4 大 MR 的隐藏成本还有一个我们在两周内就意识到的坑大 MR 会让开放式评审瞬间失效。一个 5000 行的重构 MR 推到群里除了原作者根本没有人愿意认真细看最后大概率是大家象征性点个赞然后垃圾代码悄悄合入。为了治这个问题我们把“MR 大小”也当成硬指标。超过 800 行变更的 MR 会被 Bot 贴上“变更过大建议拆分提交”的标签并提示作者先拆拦路虎。如果一个改动实在没法按模块拆也要保证它在概念上是连续的、可以按 commit 逐步 review而不是一坨混在一起。这个限制刚推行时反弹声很大后来事实说话小 MR 的评审深度和速度都全面胜出团队慢慢就自觉了。6. 从工具到习惯团队真正接受开放式评审的最后一公里工具和流程都铺好了并不代表团队就会自动用起来。开放式评审最大的阻力其实来自“每个人都得把代码拿出来被人公开讨论”的心理不适。这部分没有银弹我用过且有效的方法是一个种子队试点、一套评审礼貌规则、以及一条新人快速上手路径。6.1 先找一支“种子队”做给所有人看我没有选择一上来就全员强推而是先拉了三个认同这个方向的同事做试点。种子队的作用是产出样板健康状态的评审长什么样、怎么提问、怎么给出建设性意见这些都需要真实案例示范而不是靠文档说教。试点跑了大概两周后效果开始被其他组看见。最打动人的其实不是降下来的数据指标而是种子队里的一个小细节有个新人在公开评审中被指出一处边界问题作者立刻在评论里承认并改了代码整个过程没有私聊拉扯所有人都看得见。这种透明带来的信任感比我们开十次动员会都管用。后面其他组是主动来问能不能接入的而不是被行政命令逼着用。6.2 评审文化的几条不成文规矩工具能强制“必须评审”但强制不了“礼貌评审”。以下几条是我们团队内部逐渐形成的评审礼仪没有写进任何制度但违反会被大家私下提醒。对作者提 MR 前自己先通读一遍别把明显没写完的代码丢给评审人收到意见回消息时每个评论至少回应一句“收到已修改”或“这里我有不同思路理由是……”不要默默改完不吭声。对评审人先看整体设计再抠细节避免直接对着变量命名挑刺如果发现一个低级错误顺手把同类问题一次性列出不要一条一条发把作者逼出通告疲劳。这些规矩没有强制力但开放式评审放大了可见性之后所有人都在围观礼貌互动的价值被自然强化了。后来甚至有同事说因为在 MR 里看到了优秀的代码自己写代码时也会更注意。6.3 新人入职后的评审入门路径开放式评审对新人是一把双刃剑。好处是学习资料丰富——全组的历史评审讨论都是可读的坏处是如果没有人带新人连“怎么提问”都不会很容易在旁边看得一头雾水。我们的应对方式是给新人安排一个“评审影子任务”入职第一周不写代码而是挑三个与自己业务相关的小 MR以观察者身份全程跟踪评审讨论然后把“为什么这样改”“评审关注哪些点”写成总结。第二周开始由导师把一个低风险 MR 分配给新人做联合评审评论先写给导师看导师把关后再发出去。这样两周下来新人基本就掌握了在这套公开流程里的基本姿势。提示开放式评审最值钱的功能不是抓 bug而是它天然成了一本持续更新的团队技术文档。只要把 MR 里的高质量讨论定期沉淀成常见决策记录后续新人的学习路径会越来越短。从工具到习惯再习惯到文化这个项目最让我意外的收获是评审周期降下来只是开始真正改变的是团队对代码的协作方式。代码不再是“你写的我不管”而是每个人都愿意以旁观者的身份看进去因为反正所有讨论都是公开的与其沉默不如认真参与。这套方法不复杂平台用现有的 MR 能力Bot 只做轻量辅助真正的重点始终是机制设计和团队意愿。如果你的团队也有评审流于形式、新人找不到人评审的问题不妨也试试把评审彻底敞开——先别追求把系统做得多完美把一层透明的底子搭起来很多问题会自己在阳光下暴露。
RELATED READING

延伸阅读

更多一线实战笔记与深度复盘,助您持续精进