前端团队做 Code Review 这件事喊了很多年真正落地到每天都严格执行的团队其实很少。我自己带前端组那会儿最头疼的也不是功能做不完而是每次拉 Review 都像求人办事——有人不点开看、有人点开说句LGTM就完事、有人跟写代码的吵起来。后来我们花了大半个季度把一套 open-code-review 的流程彻底跑起来才慢慢感觉到Review 不是形式是团队工程质量的下限。这篇内容不是给你讲什么高深理论而是把我们前端团队从评审靠自觉到评审成制度的全过程复盘一遍。说说我们怎么定标准、怎么拆检查点、怎么处理评审里的沟通冲突以及为什么 open-code-review 这种开放式评审机制会比传统小圈子盯代码更适合前端团队。不管你是十几人的前端组还是在创业公司里一个人扛多个项目这套东西都能直接改一改拿来用。1. 先正视一个问题为什么大多数团队的 Review 都在走过场我见过不少团队把 Code Review 写在开发流程里Git 平台上也开了 MR/PR 必须有人 approve 才能合并的规则但实际效果跟没做差不多。评审的人打开 diff 看一眼没有明显报错、格式没乱就点个通过提测之后出了线上 bug一查全是评审阶段就该发现的问题。问题的根源不在人不自觉而是大多数团队把 Review 当成了合并前的一道手续而不是质量共建的过程。1.1 评审疲劳把精力消耗在了低频信号上前端代码的 diff 往往又长又杂。一个功能分支可能同时改了样式、组件、状态管理、接口请求和路由配置。评审人为了跟上节奏可能得花半小时一小时把上下文全部拉起来。人的注意力是有限的当你长期在大量琐碎的 diff 里反复看很快就会产生评审疲劳——看到绿色 diff 就条件反射想通过。这不是责任心的问题是流程设计的问题。我们后来做了一件事把提交粒度拆小一次 MR 只改一个逻辑主题。比如新增购物车数量角标和重构购物车状态管理分两个 MR不混在一个分支里。拆开之后评审人每次面对的 diff 通常只有几百行甚至几十行注意力和判断力就能集中在真正需要看的地方。这个改动立竿见影评审意见的质量直接提高了一个档次。1.2 上下文缺失没有描述和截图评审全靠猜另一个痛点就是 MR 描述写得太糊。很多开发提 MR 就写一行完成购物车功能里面改了 20 个文件。评审人看到这种描述根本不知道代码想干什么、为什么这么干、有什么限制条件只能自己对着代码猜。前端项目里这种问题尤其明显因为很多东西是视觉层面的——你改了布局、改了间距、改了动效不看页面根本不知道效果对不对。应对办法也很直接强制 MR 模板。修改了什么、为什么这样改、影响范围、自测结果、关键截图都必须要填。截图这一条对前端特别重要你贴一张本地跑出来的界面截图比写二十行说明文字都管用。后面我们还会把 MR 模板的完整示例放出来可以直接抄。1.3 面子文化评审意见没人敢提这是最微妙的一点。团队里大家天天见面经常一起吃饭一起加班到 Review 的时候发现对方的代码有明显问题多数人第一反应是当众提出来会不会不太好于是很多问题就在这种算了算了的心态里被放过去了。open-code-review 强调的恰恰是打开这层窗户纸。它把评审从私下里的口头交流变成公开的、格式化的、对事不对人的技术讨论。我们在团队里立了一条规矩评审里所有问题都记录在 MR 评论区公开可见禁止私聊说你那个代码有问题我帮你改了之类的话。一旦所有问题都公开化、书面化大家反而能把焦点放到代码本身而不觉得是人身攻击。这个转变花了我们大概一两个月但效果是长期性的。2. Review 前的工作把提 PR当成一次正式汇报很多人觉得评审是评审人的事提交方只要把代码写出来就行。这其实是最根本的误解。评审质量的上限在代码提交的那一刻就已经定了。提交方有没有认真自测、描述写得清不清楚、变更范有没有收敛——这些决定了评审人能发挥多少价值。所以 open-code-review 流程的第一步不是列评审点而是训练所有人把 MR 当成一份要拿去汇报的材料来准备。2.1 MR 描述模板描述里写什么评审者才不用猜我们团队内部沉淀了一套 MR 描述模板固定了几大块内容大家提合并请求的时候照着填。这里直接分享一下## 背景与目标 说明这个变更要解决什么业务问题或技术问题 ## 变更内容 列出主要的改动模块和文件说明每个点改了什么 ## 设计思路与取舍 为什么要采用这个实现方式有哪些替代方案被放弃及原因 ## 影响范围 会影响哪些页面/组件/接口是否需要后端或其他团队配合 ## 自测记录 - 正常路径xxx - 边界条件xxx - 浏览器兼容Chrome/Safari/Firefox 实测结果 ## 截图 前端项目必填核心界面前后对比 ## 其他说明 如依赖项升级、潜在性能考量一开始推行这个模板的时候组里有人嫌麻烦说又不是写论文。但坚持了一两个月之后大家逐渐感受到了好处评审人上手快沟通成本低以前需要来回追问的事情在描述里都写了。慢慢地把 MR 写清楚反而变成了一种技术表达能力的体现。2.2 提交前自测清单先在本地把低级问题干掉每个人提交前应该按下面的清单过一遍否则 MR 提出来就是在浪费评审人的时间功能路径是否完整跑通而不是只看自己改的那一行关键交互在窄屏、宽屏下的表现是否检查过有无明显 console 报错有无未处理的异常用到的接口数据字段是否和后端实际返回一致改动是否引入了多余依赖或无用代码TS 类型是否通过检查有没有大量使用 any 代替自测这个环节本质上是把人工评审的时间留给真正需要思考的问题——比如状态设计是否合理、性能是否达标、边界是否处理。如果低级问题一堆就拉评审评审人大多会直接失去耐心后面的有效意见也没了。2.3 小步提交一次 MR 只做一件事前面提过拆小 MR这里补充一下实操细节。我们内部要求单个 MR 的改动文件尽量控制在 20 个以内、核心逻辑改动控制在 500 行左右。一旦超过这个量级评审人会本能地产生抗拒心理更倾向于扫一眼就通过。拆 MR 跟写代码一样需要能力。你得规划好分支策略功能 A 的分支和功能 B 的分支相互独立先后合并而不是在同一分支里堆叠。这里可以简单列一下我们现在用的方式# 基于 main 拉一个功能分支 git checkout -b feature/cart-badge # 完成第一个逻辑主题后提交推送到远端 git push -u origin feature/cart-badge # 合并后再基于最新 main 拉下一个功能分支 git checkout main git pull git checkout -b feature/cart-state-refactor这套做法起初会让人觉得分支变多、流程变重。但两个 MR 各 200 行比一个 MR 400 行评审质量和速度完全不一样。从时间成本看前者是省时间的。3. 前端评审的核心检查面拿到一个 MR 到底该看什么这一节是全文最核心的部分。很多人评审前端代码只能看出语法对不对变量命名好不好但前端项目的坑往往不在语法层面而在数据流、状态设计和交互细节里。open-code-review 的流程之所以有效是因为我们把评审做成了分层的检查——每一层都有明确的问题清单评审人按清单过一遍就能覆盖大多数风险点。3.1 第一层变更意图与 diff 范围拿到 MR先别急着看代码细节先看整体 diff 是否和提交者描述的目标一致。常见的问题是为了改一个按钮颜色把整个组件的逻辑也重构了。这种顺手改其实很危险——你以为的无意改动可能把另一个功能的预期破坏了。这一层的检查点包括变更是否局限于 MR 描述提到的模块是否有无关文件被改动比如自动生成的配置文件、锁文件是否有删除的代码是被其他模块引用的被乱改的锁文件这个坑我们踩过好几次。有人本地用不同版本的包管理器跑了一下锁文件就被大幅改动混进 MR 里一起提上来评审人一旦没注意合并后队友拉代码就可能装出不同的依赖树线上跑出诡异问题。所以现在我们对锁文件的改动非常敏感。3.2 第二层状态管理与组件数据流前端项目里最容易出潜在 bug 的就是状态滥用。评审时要追问几个问题这份数据真的需要放进全局状态吗还是组件本地 state 就够同一个领域的数据有没有出现在多个 store 里互相需要同步组件从 props 拿数据和从 store 拿数据来源是否统一有没有直接修改 props 或外部 store 的操作我见过太多代码把接口返回的数据塞进全局 store组件里又通过 computed 派生一堆字段到最后改一个字段牵一发动全身没人说得清数据流是怎样的。评审时不能只关注这段代码能不能跑还要追问这个状态设计未来扩展性怎样。如果发现状态放错了层级、派生逻辑绕来绕去哪怕功能正常也应该提出来——这种问题迟早变成技术债而且越晚处理越贵。3.3 第三层UI 实现与样式正确性前端不同于后端的很重要一点是代码逻辑过了视觉效果不一定对。样式相关的检查不能靠读代码必须看截图、看实测。我们整理了一份样式检查清单响应式断点是否正确窄屏下有没有横向溢出组件在相邻容器内的间距是否符合设计规范而不是硬编码魔数字体、字号、行高是否继承全局设计 token而不是自定义了一套交互状态hover/focus/disabled/loading是否都实现完整是否有深色模式适配问题如果项目支持样式检查最容易被忽略的是 focus 态。很多开发者做完点击、hover却漏掉键盘操作时的焦点样式这在无障碍检测里是硬伤。评审时如果看到按钮、输入框没有对应的 focus 样式可以直接提出补上。3.4 第四层性能与渲染开销前端的性能问题在评审阶段就需要预判。常见的高频问题大列表渲染是否使用了虚拟滚动还是直接把几千条数据全量渲染组件更新频率是否过高有没有不必要的重渲染事件监听和定时器是否在销毁时清理图片资源是否做了懒加载体积有没有压缩依赖项的体积、构建产物的大小是否有明显增长评审时注意一个点初级前端容易追求把所有东西都做成响应式结果一个数组变化就触发大范围重渲染。评审时可以打开 Performance 面板粗略看一眼渲染耗时如果页面明显卡顿那不管代码逻辑多优雅都要打回优化。3.5 第五层类型与边界条件TypeScript 已经是前端标配了但 TS 的类型检查通过不代表逻辑边界处理正确。评审时要专门看看这些分支接口返回的数据是否为空的处理嵌套对象是否存在空指针风险列表边界空数组、长度为 1、数据量超级大表现是否正常用户快速重复操作时有没有防重复提交时间、金额等格式化边界时区、精度、负数type 层面还有一个高频痛点滥用 any。很多项目里接口响应类型没定义完整前端拿过来直接 any 掉等线上报错才到处找字段来源。评审时应定一条硬规则代码里不允许出现显式 any除非有注释说明理由并经过评审确认。3.6 第六层安全与敏感信息前端的安全责任常被低估。评审时重点关注下面几个方向代码中是否有明文硬编码的密钥、token、账号信息用户输入是否经过转义是否存在 XSS 注入风险特别是 v-html 之类的动态渲染URL 拼接是否使用编码处理权限控制是否只依赖前端路由守卫还是后端同样校验前端权限校验只是体验层的分流不是真正的安全边界。评审时如果发现某个接口的敏感操作只在前端做了菜单隐藏后端没有鉴权必须提出增加后端校验。下表汇总了上面六个层面和它们的检查重点评审人可以直接打印出来当指引检查层级核心关注点常见脏点变更意图改了什么、是否超出范围、是否有误改动无关文件、锁文件混乱状态数据流全局/局部状态分层、数据来源统一滥用 store、重复状态源UI 与样式响应式、设计 token、交互态、无障碍硬编码样式、缺 focus 态性能渲染渲染成本、事件清理、资源体积大列表全量渲染、内存泄漏类型边界空值、边界值、重复操作、格式化滥用 any、空指针、未防重复提交安全合规敏感信息、XSS、鉴权边界硬编码密钥、前端权限当安全4. 评审反馈的硬规则怎么提意见才能不伤和气、不流于形式评审意见能不能被接受、能不能转化为有效动作很大程度上取决于意见的表达方式。前端团队的沟通通常比较直接但代码评审这件事还是得讲究方法。粗暴地甩一句这段代码写得有问题等于没说温和到不敢提等于白审。我们在实践中沉淀了几条反馈硬规则。4.1 反馈分级什么必须改、什么可以后续、什么只是疑问每条评论都应该有一个明确的级别前缀没有级别的评论是无效评论。我们内部约定三级P0阻断功能错误、安全漏洞、明显性能问题必须修改后重新评审才能合并P1建议当前不修改也可以用但后续大概率引发问题最好在本次修掉P2可选风格、命名、后续优化方向的建议可以记录为后续 TODO用分级的好处是提交方能快速判断每个意见的处理优先级评审双方不再为这算不算问题争论半天。实践下来P2 占多数的时候评审氛围普遍更轻松因为大家知道 P2 不是强制修改的要求用不着防御心理拉满。4.2 用提问的方式给反馈而不是下命令给意见时建议用为什么/是否/有没有考虑的句式而不是你应该/你必须/写成这样不行的句式。比如命令式反馈这个组件不应该直接改 store 里的数据。提问式反馈这里直接改 store 里的共享状态会不会影响其他页面的展示要不要考虑用事件触发父组件更新两种表达想要传达的内容可能一样但接收方的感受完全不同。提问式把决策权留给提交方让对方自己去推导问题的存在性。一旦他自己得出了应该改的结论这次讨论就变成了主动的技术成长而不是被强迫改代码。4.3 评审人不应该帮忙改代码更不建议直接合并代修这一点每个团队都容易踩。看到别人的代码有个小瑕疵评审者顺手改了然后说我帮你修了一下。这种操作看着是热心实际坏了规矩代码库里出现了评审者写但提交者不完全理解的代码出了问题责任说不清。正确的做法是只描述问题和建议方案由提交方自己修改。我们也遇到过评审来回好几轮、改动量已经很大、双方都有点焦虑的情况。这时候可以用一套固定的动作收尾提交方把历史评论逐条回复标记每个 P0/P1 的处理结果评审者逐条确认或继续追问。把节奏掌握在逐条确认这个动作上能避免讨论发散、跑偏。4.4 合并条件要明确谁有权合并什么状态才能合并open-code-review 流程里必须明确合并权限。建议至少一个核心评审者 approve 且无未处理的 P0 级评论时才能合并。关键或公共模块比如公共组件库、构建配置、权限逻辑可以设置 Code Owner改动这些文件必须由指定 Owner 审批。千万不要让开发者自己 approve 自己提交的 MR这条底线从流程开始就要守住。5. 落地一套 open-code-review 流程具体到每个环节怎么做前面讲的都是原则和检查点这节我们直接把整个流程串起来从提交前到合并后每个环节给出能照做的动作定义和时间上限。这套东西在不同团队规模下都需要裁剪但骨架是通用的。5.1 流程全景从编码到上线的七个动作环节动作负责角色时间要求提交前本地自测 lint 类型检查通过提交者无提交流程按 MR 模板填写描述附截图和自测记录提交者无自动检查CI 运行单测、构建、代码扫描机器10 分钟内完成人工评审按六层检查点给出分级意见评审者24 小时内给出首轮意见修改反馈提交者逐条回复修复 P0/P1提交者48 小时内完成复核确认评审者逐条确认必要时循环评审者24 小时合并上线Code Owner 或有权限的评审者执行合并指定人确认后立即执行时间要求可以根据团队节奏调整但不能不设限。没有时限必然拖成评审马拉松大家都没效率。我们的经验是超过 3 天没完成一轮完整评审的 MR建议提交者直接在群里提醒评审人超过一周没动静的找 leader 介入。5.2 自动化闸门让机器先把低级问题挡在外面人工评审资源有限得用在刀刃上。提交前置钩子和 CI 阶段能自动检查的事绝不让评审人看。在项目仓库里加一套 pre-commit/pre-push 钩子配合 lint-staged 做增量检查基础配置长这样{ husky: { hooks: { pre-commit: lint-staged, pre-push: npm run lint npm run type-check npm run test } }, lint-staged: { src/**/*.{ts,tsx,vue}: [eslint --fix, prettier --write] } }CI 阶段在跑完 lint 和单测之后再加一轮构建产物大小对比把新增依赖导致体积暴涨的 MR 直接拦下来。这样评审者拿到手里的 MR基本没有低级格式问题他们集中精力看逻辑和数据流即可。5.3 评审人分配怎么保证每一行代码都有人看小团队里天然就是互相评审不太需要复杂的分配机制。但超过十个人的组就要考虑评审人的负载均衡。我们用的方式很简单按照业务模块指定 primary reviewer 和 backup reviewerprimary reviewer 休假定事backup 顶上每个 MR 至少找一名对这块代码不熟悉的人参与评审也就是新鲜视角新鲜视角这条我们坚持了很久。只让同一批老手互相评审容易形成思维定势——大家共享同一套盲区。拉一个不熟悉该模块的人进来反而能问出很多为什么这样写的基础性问题逼着提交者重新审视设计。6. 评审标准的版本化让团队的共识持续生长Review 流程跑起来之后还有一个关键环节把评审中发现的高频问题沉淀成标准并且让标准跟着项目一起演进。很多团队的 Review 之所以越做越形式化是因为检查项永远停留在团队建立初期的几条项目在变、技术栈在变、团队在变标准却原地不动。6.1 建立前端 Review 高频问题库每个季度我们都会把评审评论全部过一遍挑出出现频率最高的 10~20 个问题整理成一份《前端常见评审问题清单》。这份清单不是摆设而是新人的参考手册也是评审人自查的备忘录。举几个我们进入清单的例子组件卸载后异步回调仍更新状态导致内存泄漏列表 key 使用 index 导致渲染错乱接口请求无错误处理loading 状态永远不关闭只考虑了成功路径没有设计空态、失败态、超时态深层对象更新用扩展运算符导致层级丢失事件节流或防抖缺失滚动或输入时频繁触发计算这些问题单个看似乎都不大但每个都造成过线上事故或用户投诉。把高频问题库建立起来之后评审人不用每次重新思考对应清单检查即可。新人写代码前先看一遍清单也能提前规避大部分常见坑。6.2 让标准可讨论、可更新而不是永不变化清单要定期修订。我们每个季度安排一次技术评审复盘会大家把最近遇到的典型问题、新工具链带来的新检查点、团队风格调整等因素加进清单。比如我们引入 React 18 和并发渲染相关 API 之后就专门加了一条并发模式下是否有重复调用风险的检查项。再比如引入设计系统之后加了是否符合组件库 api 设计规范的检查项。这个过程让评审这件事持续保持新鲜感不会变成每年一样的八股文。如果团队里有人对某条评审标准有不同看法应该直接提出来在公开讨论中达成新的共识。评审标准不是某个 leader 一锤定音而是团队共同维护的文档。这条听起来简单却是让评审真正融进团队文化的关键。6.3 数值指标评审这件事本身也需要被量化没有度量就很难改进。在每个迭代结束后我们会拉一组简单粗暴的指标平均首轮评审时长反映评审响应是否及时评审驳回率有 P0/P1 意见的 MR 占比反映提交质量人均评审评论数反映评审参与度每周期评审引发的 bug 数反映评审机制的兜底效果这些指标不是用来考核某个人而是用来监控整个流程是否健康。比如评审驳回率突然下降可能是提交质量真的变好了也可能是评审变松了。需要结合评论内容和时长综合分析不能只看一个数字。7. 写在最后Review 文化不是一堆规则而是团队对代码的尊重这套 open-code-review 流程在我们团队落地半年之后最直观的变化不是 bug 变少——bug 确实变少了但更明显的是大家对提交代码这个动作的态度变了会认真写描述、会提前自测、会在提交前用别人的视角审视一遍自己的代码。代码不像从前那样只是写完就急着推上去而是多了一层我要把它展示给同事至少要经得起看的自觉。如果你也想在团队里推行类似的东西我的建议是从小的切口开始先定 MR 模板和 P0/P1/P2 分级这两条立竿见影。等大家习惯了再逐渐加检查点、铺自动化、做复盘。不用一次把整个体系硬塞到团队里否则只会遇到反弹。最后再分享一个小经验Review 里发现的问题哪怕是 P2也不要只丢一句话就完。尽量写清楚问题发生的场景、可能的触发路径和参考的改进方向。一条好的评审意见本质上是给同伴的一份技术指导——认真写评论的团队代码水平和协作氛围都会肉眼可见地向好。
