12-代码审查CR/PR流程:团队合并请求、代码质量卡点规范
12-代码审查CR/PR流程团队合并请求、代码质量卡点规范大家好我是黒漂技术佬。今天这篇是整个系列的最后一篇聊一个听起来高级、做起来得罪人的话题——代码审查Code Review简称CR。很多团队把CR当成走过场MR来了瞄一眼点通过完事。这样的CR连橡皮图章都不如真不如不做。今天我们聊聊如何让CR真正发挥作用——既能拦住bug又不拖累开发效率。一、CR/PR流程设计先说概念澄清。GitLab叫MRMerge RequestGitHub叫PRPull Request两者本质相同——把功能分支的改动请求合并到目标分支。一个标准CR流程分四步提交MR → CI自动化检查 → 人工代码审查 → 合并第一步提交MR开发者在功能分支上完成编码后向目标分支通常是develop发起MR。MR提交时必须填写模板。第二步CI自动化检查MR一旦创建CI流水线自动触发。这一步不需要任何人操作机器先做一轮把关。具体卡点是什么下面细讲。第三步人工代码审查CI通过后指定至少一位Reviewer审查代码。审查者逐文件阅读diff发现问题直接在对应代码行添加评论。审查结论有三种Approve通过、Comment需讨论、Request Changes要求修改。第四步合并所有Reviewer都点了Approve后由MR作者自己执行合并。合完后删除源分支保持仓库整洁。二、MR模板配置没有模板的MR就是开盲盒。团队应该强制所有MR使用模板。GitLab仓库根目录创建.gitlab/merge_request_templates/default.md## 需求背景 !-- 这个MR解决什么问题关联哪个Issue -- ## 改动内容 !-- 简要描述本次改动的技术方案 -- - [ ] 新增功能 - [ ] Bug修复 - [ ] 重构 ## 自测情况 !-- 列出你自测过的场景 -- - [ ] 单元测试通过 - [ ] 本地接口测试通过 - [ ] 边界条件已验证 ## 影响范围 !-- 本次改动影响了哪些模块/接口需要通知哪些同事 -- ## 检查项 !-- 请自查以下项目 -- - [ ] 代码无hardcode的IP/密码/Token - [ ] 日志打印不会泄露敏感信息 - [ ] 数据库变更已附带SQL脚本 - [ ] 接口文档已同步更新 - [ ] 截图/录屏前端类MR必须 ## 补充说明 !-- 任何Reviewer需要了解的特殊情况 --设置方式Settings → General → Merge requests → Templates → 选择你的模板文件。配置好后每次创建MR时模板自动填充开发者只需填写具体内容。三、代码质量卡点好的CR流程不会依赖纯人工审查机器先做第一道防线。卡点1编译检查最基础不过。代码推上去连编译都过不了就不配进入人工审查。CI第一步就是mvn clean package或npm run build。卡点2静态代码扫描我们使用SonarQube作为代码质量门禁。配置Quality Gate重复代码率 3%代码覆盖率 60%新增代码覆盖率 80%阻断级别issues 0严重级别issues 5代码异味评级至少为ASonarQube不通过MR不能合并。这是硬规则没有例外。卡点3单元测试覆盖率CI流水线自动跑全量单元测试。配合JaCoCo生成覆盖率报告。低于60%的模块构建直接标红。新增的代码覆盖率要求更严格——低于80%直接打回。我们不是为了覆盖率好看才做的。覆盖率高不一定没bug但覆盖率低的代码一定有很多没有验证的路径。卡点4安全扫描使用GitLab自带的SAST静态应用安全测试自动检测常见安全漏洞SQL注入、XSS、硬编码密码、不安全的反序列化等。卡点5代码风格检查配置CheckstyleJava或ESLint前端。CI中任何风格违规都阻止合并。这不叫形式主义——统一风格能显著降低CR的认知负担。四、人工审查Checklist机器审核到位了人工审查才能真正关注业务逻辑。审查者需要逐项检查功能正确性代码实现了需求吗有没有遗漏的边界条件安全性有没有注入风险用户输入是否做了校验性能有没有不必要的循环数据库查询有没有索引支持可读性变量名有没有意义复杂逻辑有没有注释可维护性有没有过度设计有没有重复代码错误处理异常有没有被恰当处理错误信息对排查有帮助吗测试覆盖关键路径都有单测吗一次CR的黄金法则审查时间不超过1小时改动行数不超过400行。超过这个尺度审查质量急剧下降。大功能拆成多个小MR是CR效率的关键。五、审查评论的沟通艺术很多人对CR有抵触情绪觉得被挑刺。说白了是沟通方式的问题。好的评论这里用StringBuilder拼接SQL可能有注入风险建议改为参数化查询。参考UserDao.java:145的写法。差的评论为什么写这么蠢的代码改了。好的评论有三个要素指出具体问题说明为什么是问题给出修改建议或参考。审查者的心态CR不是为了证明谁聪明而是两个工程师一起确保代码不出问题。发现问题就说但说完一定要给建设性建议。MR作者的心态代码被人指出问题是好事。每一个被发现的bug都是没有流入生产环境的bug。如果别人在你的代码里找到了10个问题你应该开心——因为你不用半夜爬起来修了。六、团队CR文化建设工具和流程只能解决一半的问题另一半靠文化。原则1速度优先团队约定工作日MR提交后4小时内必须有人开始Review。如果Reviewer太忙作者可以直接在IM上提醒。拖得太久会导致上下文丢失和频繁冲突。原则2小步快跑单次MR控制在200-400行变更以内。超过就拆分。小MR审查快、风险低、冲突少。原则3Review一切哪怕是改一行配置文件也要走MR。没有例外。这不是麻烦而是确保至少有两个人的眼睛看过程序里的每一个字符。原则4新人友好新人 первые几个MRReviewer要花更多时间。不光指出问题还要解释原因引导他去看相关文档和代码规范。这些时间投入在后期会收到十倍回报。原则5持续改进每月的团队回顾会上我们会翻出上个月被Review打回的典型问题做成反面教材。这不是批斗而是集体学习——同样的坑大家只踩一次。代码审查这事本质上是一笔投资。前期投入时间和精力换来的是更少的生产bug、更干净的代码库、更快的团队成长速度。当有一天你看到一个同事在你的Review评论下写好的学到了你就知道这事做对了。整个版本管理系列到这里就结束了。从固件OTA到小程序发布从Git仓库搭建到代码审查——每一环都是多端项目的生命线。希望这些实战经验能帮到你。我是黒漂技术佬代码平安我们江湖再见