code review 是技术团队在上线需求前的必经阶段，不同的团队，code review 风格各有不同，本文总结了笔者之前的 code review 经验，形成一套较为详细的规范。

目的

• 提高项目质量，提前发现缺陷，减少线上问题的发生
• 提高代码质量，便于后续的维护
• 增加其他人对代码的熟悉度
• 促进知识的传播，提供自己和他人的编码能力

时机

常规的开发流程通常是这样的

• 三次需求评审看似有些重叠，实际上是在为需求的可行性和完整度保驾护航，不合理、无法实现的需求会在这个过程中被剔除，需求不详细的部分会在多次的评审中得以补充
• 技术评估在需求五稿评审后就可以开始启动了，这个阶段可能会涉及到一些验证性代码的编写
• 技术评审一方面可以让其他同学可以了解到当前需求的技术实现方案，另一方面可以让更多的人参与到技术设计中，讨论并最终得出最佳的技术方案
• code review 的时间：原则上，每一行代码都需要被 review 后才能够被合并，所以，cr 是从技术评估开始，到需求上线结束，贯穿了几乎整个研发的生命周期

名词释义

角色

名称	角色
author / committer	MR 的提交者
reviewer	review 代码的人
owner	仓库管理者

黑话

缩写	全称	含义
MR / PR	Merge Request / Pull Request	合并请求
LGTM / SGTM	Looks Good To Me	可以合并
WIP	Work In Progress	功能尚未完成
PTAL @xxx	Please Take A Look	提示别人来看一下
TL;DR	Too long; Didn’t Read	太长了，懒得看
ASAP	As soon as possible	尽快
np	no problem	没问题
AFAIK	As far as I know	据我所知
ACK	Acknowledgement Character	确认
BTW	By the way	顺便
FYI	For Your Information	供参考
a.k.a.	also known as	又称作
CR	Code Review	代码审查
CL	Change List	这次改动的内容
NIT	nitpick	鸡蛋里挑骨头，用来表示建议性的修改（如果能改成这样更好）
Sug	Suggestion	建议，带有这个标志优先级高于 NIT，低于不加标志（不加任何标记表示必须要关注）

注释

名称	含义
TODO: [说明]	在标识处有功能代码待编写，待实现的功能在说明中会简略说明。
FIXME: [说明]	标识处代码需要修正，甚至代码是错误的，不能工作，需要修复，如何修正会在说明中简略说明。
XXX: [说明]	标识处代码虽然实现了功能，但是实现的方法有待商榷，希望将来能改进，要改进的地方会在说明中简略说明。

author 视角

• 作为这段代码的 owner ，你需要确保自己的代码被尽快的合并，可以使用的手段包括但不限于：
  • 设置 MR 的 assignee，他会收到一条明确的通知
  • 邀请必要的 reviewer
  • 在群里发出你的 mr 链接，让更多人知道你的 mr 需要被 review
  • 在群里 at 或 私聊 相关责任人（partner, feature owner, mentor, leader）
  • 通常 review 是大家的第二优先级工作（第一优先级是开发自己的需求），如果你的功能不是特别急着合入，尽量不要打断其他人的工作，保证 reviewer 知晓即可
• 确保 CI 通过，CI 中应该包含
  • 依赖安装
  • 类型检查
  • 单元测试
  • 代码构建
  • 推荐增加：代码重复率检查、注释覆盖率
• 确保自己对代码进行了一个完整的 reveiw，可以减少很多低级错误的发生
• 代码行数不宜超过 300 行
  • 行数过多的 MR review 起来会很疲惫，最后导致效果差，有问题没被发现，上线之后背事故💔
  • 在做技术设计时将模块拆解好通常可以避免「大 MR」 的出现，精心的模块化设计正是体现了开发者的「匠心精神」
  • 如果出现了设计问题，代码行数越少越容易转变方向
  • 更容易被合并（review 起来更快），回滚（git 操作更简单）和 cherry-pick
  • 如果你的 MR 没搂住真的有上千行，约上几位同事找个会议室线下 review 吧，正式的场合可以提升 review 的质量，更多人参与可以降低疲惫感
• 如果 comment 描述的不清晰，请不要妄加猜测，坦诚清晰的让 reviewer 描述清楚即可
• 遇到无法解决的冲突，可以引入你信任的「第三者」
• 提交 MR 的时间：颗粒度按照最小模块完成即可提交 MR，MR 最好是能够在当天被提交，当日被合并

reviewer 视角

• 在可能的情况下，确保其他人的代码能够尽快被合并，更慢的 cr 可能会导致
  • 后续模块的开发受阻
  • 自测联调进度受阻
  • bug 日清目标无法打成
  • 上线 delay
  • 精神崩溃☢️
• 保证 review 的严谨程度，如果上线时间非常紧急，可以适当的降低门槛，但是对于上线时间非常紧急需要有很明确的判断，如果你不能作出这样的判断，请将问题抛给 leader（背锅侠）
• 大胆质疑，充分讨论，遇到没看懂的可以提问（鼓励 author 增加注释或改进逻辑，通常你看不懂的别人也看不懂🙈），遇到觉得不对的坦诚清晰指出来，解释清楚你的想法，明确的表达可以减少很多无意义的猜测，明确的解决方案可以减少 author 盲目的寻找解决问题的时间
• 客观的对问题进行评论，不要携带个人色彩和情绪
• review MR 的时间：大块的时间需要支持业务需求，碎片化时间来 reveiw 代码是个不错的选择，例如：
  • 早晨坐到工位之后
  • 午餐前
  • 午餐后
  • 晚餐前
  • 晚餐后
  • 下班前
  • 代码写累了🏋️

心理建设

为了做好 code review，我们需要告诫自己

1. 保持风度

• 时刻谨记我们是成年人，需要在公共场合下「保持风度」。保持风度意味着在评论他人代码时注意自己的语气，被指出问题通常是感觉不好的，注意语气可以让他人更容易接受你的想法；同时被别人指出问题的时候也要尽量积极回应，从正向的角度进行解释或改正，对事不对人
• 类似在聊天中使用表情包，多在 CR 时使用黑话或emoji可以让你的语言变得不那么强硬
• 请不要吝啬你的赞美，看到你觉得好的代码，请疯狂点赞👍并截图晒出来

2. 善意假设

• reveiwer 的评论都是在为项目的质量负责
• 有利于 author 的成长
• 经过严格 review 的代码可以减少 author 的线上事故，这是在为公司级别作出的重大贡献💯
• 那些你听上去感觉不舒服的词并不是 reviewer 的本意

3. 听人劝，吃饱饭

• 当你觉得 review 提出的评论与你自己的设计不符时，不要一味的坚持自己的想法，试着从他人的角度理解一下问题（同理心），进行必要的线上 or 线下讨论，你会发现自己会得到很快的成长

4. 出来混总是要还的

• 尽量避免“未来修复”，因为通常未来不会修复。我们更应该做的是立足当下，如果简单的花点时间就可以将小问题改正，那就花点时间做吧（哪怕是耽误了午休或按时下班），不然只是在为未来埋下了更大的坑，然后期望下一个承接这段代码的人不知道你的家庭住址🪓
• 留下一些注释，如果你不确定这段注释是否必要，那么请保留，大部分人通过代码理解逻辑的速度是慢于读注释的，而且可以避免出现「上个月上帝和我知道这段代码的用途，现在只有上帝知道了🙈」

5. 群众的眼睛自然雪亮

• 遇到 author 和 reviewer 各执一词的情况时，不要试图说服对方，可以 at 其他相关的人或者你觉得有权威的人进来讨论，如果大多数人的意见打成一致并且与你的想法相悖，请放下心中的执著，你会发现世界是那么的美好~

6. 坦诚清晰

创造一个坦诚清晰的 CR 氛围很重要，在这样的氛围下，大家都愿意提出建议，接受建议，但这不是一蹴而就的，为了创造一个和谐的氛围，从自身做起吧 以下观点来自《不拘一格》

• 目的在于帮助：秉持一个帮助他人的初心
• 反馈具有可行性：评论时记得提出可行性建议
• 感激与赞赏：对于你觉得有价值的评论，请点赞
• 接受或拒绝：明确结论，对于有价值的积极接受，对于无价值的内容拒绝并提出让人信服的观点

最佳实践

分支命名规范

• 分支有明确的含义标头，例如 feat/add-xxx hotfix/text-error
• 分支中连字符使用 -

分支管理

• 个人分支通常从 release 分支中迁出，完成开发后向 release 分支提交 MR
• 提交 MR 后，通常会有人给你进行评论，此时的修改可以按照内容来提交 commit，也可以直接 amend (amend 的好处是可以将功能聚合到一个 commit 中)
• 提交 MR 后，通常不会立刻被合并，此时可以基于上一个 feat 分支或 release 分支新建一条分支来做另外一个需求，从哪个分支迁出取决于对哪个分支有依赖
• 当从 release 分支迁出分支并且在你的 feat 分支合并之前有其他分支已经合入，此时你的分支是落后 release 分支的（此时可能会有冲突出现），解决方案是使用 rebase 命令

尽量不要使用 merge

merge 会让分支结构变得凌乱，不便于进行回滚和 cherry-pick；merge 一般仅在 release 合并回 master 时使用，这种情况下使用 rebase 非常容易引起心态爆炸💥，除此之外请使用 rebase。

例：假设你的分支是 feat/add-feat-1，在基于 release/1.0 迁出后开始进行功能开发，此时有另外一个功能已经合并到 release/1.0 中并且在 feat/add-feat-1 合并到 release/1.0 时有冲突，此时两种解决冲突方式带来的 git 提交历史的区别：

merge 的提交历史

rebase 的提交历史

commit message 规范

• 每个 commit 只解决一个问题
• https://gitmoji.dev/ 在 commit 中添加一些有趣的元素
• commit message 内容规范：[类型]:标题

fix:      bugfix
hotfix:   线上紧急热修复
feat:     新feature 
docs:     修改文档
test:     修改测试
refactor: 重构
chore:    修改Makefile/build.sh/gitlab-ci.yml/workflow.yml etc.

合并时机

代码最终被合并需要有 author 来执行而不是 reviewer，当然，author 可以在提交 MR 之后选择 auto merge。

Code Review 类型

线下会议

• 线下会议通常每周一次，每次一个小时
• 内容为一个人主持 review 的过程，对当前存在于代码仓库中待 reveiw 的 MR 进行 reveiw
• review 的过程中可以一起评论和讨论最佳实践
• 好处：
  • 便于展示 review 本身的一些内容，比如不同人 review 是会关注哪些部分，方面新同学学习，也方便大家统一思维方式
  • 每周一次轮值主持人，可以深入体验 review 的过程，并且这个过程中有会议中其他人协助

code reveiw 大赏

• 形式为讨论组
• 内容为针对 review 中具体问题的讨论
• 应用场景
  • 出现无法说服对方的情况，可以将问题抛在讨论组中
  • 觉得写的好的代码，可以贴出来大家一起看
  • 觉得写的不好的代码，可以贴出来并讨论这里面的共性问题，给出解决方案

线上日常 review

• 日常的线上 reveiw 是应用最多的 review 场景，在收到 review 通知并且此时没有优先级较高的事情时就可以开始进行 review 了 code review 最佳实践收集
• 线下会议或讨论群中通常会有一些最佳实践或集体共识，这些内容会被统一收集到一篇文档中维护

code reveiw 数据收集

• 收集每周+每月的 code review 数据
• 周报中展示每周的数据情况，重点是 comment 数
• 目的是从数据层面上体现 cr 的量，激励大家积极参与到 cr 中

常见问题

代码设计不好，上线前才被指出来

• 首先，提交代码的颗粒度需要控制好，这样问题可以在更早期暴露
• 提交代码的时间持续整个开发周期，不要在上线前才提交 MR
• reviewer 需要注意 review 效率，一个 MR 最好不要超过两天被合并，尽量日清
• author 在时间合适的情况下可以选择更直接的方式邀请 reviewer

上线后一些低级的逻辑问题导致线上事故

• reviewer 在 review 时需要严谨对待，每个人都养成严谨的习惯，自己的代码在上线时也会更有信心
• author 在提交 MR 是一定要以第三者的角度 review 一遍自己的 MR

突然被拉去 review 其他人的代码，导致自己的工作被拖延

• 需要平衡好优先级，如果其他人的 MR 优先级确实很高（block 测试进度、上线进度等），可以适当放下手头的工作；如果手头的工作不能放下，那就把这个问题抛出来吧，at leader or mentor 都是一个不错的选择
• 注意：此时千万不要友情 approve，带来线上问题或者为项目留下隐患都得不偿失