阅读 1803

谷歌都在用的 code review 技巧

前言

最近读了 几篇code review 的文章:

Google 工程师 Michael Lynch

怎么样使code review 做的更好

读了之后,感触很深,发现我所任职的公司对于 code review 的重视程度还远远不够,很多时候都把它视为一件麻烦事。

任何软件都是协同开发的,所以 code review 非常重要,它可以帮助你减少代码质量问题,提高开发效率,提升稳定性,同时还能保证软件架构的稳定性,防止代码结构被恶意破坏导致难以维护。

摘要

概念剖析 - 什么是 code review?

首先 code review 是一个活动,从最简单一个人读小伙伴的代码开始,到团队在办公室内一起读,一起剖析代码,这都是可以叫做‘code review’

角色划分

从上图我们可以看到一共有两种角色:

  • reviewer - 一起阅读代码的人
  • author - 写代码的人

code review 的覆盖范围

  • 好的 code review 会检查代码的正确性、测试覆盖率、功能变化、是否遵循代码规范与最佳实践、可以指出一些较为明显的改进点,比如难以阅读的写法、未使用到变量、一些边界问题、commit 数量过大需要拆分等等。

  • 良好的 code review 会检查引入代码的必要性,与已有系统是否适配,是否具有可维护性,从抽象角度思考代码是否与已有系统逻辑能够自洽。

如何判定是否可以 merge

  • 好的 code review 会轻易通过那些开放式 PR,至少在其被得到充分讨论前,但每个 Review 者对自己关注的部分完成 Review 后需要进行反馈,无论是 “看起来不错” 或者用缩写单词 “LGTM”,之后需要有明确的跟进,比如通过协作软件通知作者进行进一步反馈。
  • 良好的 code review 实际执行中会更加灵活一些,对于一些比较紧急的改动会留下改进建议,但快速通过,让写代码的 author 通过后续代码提交解决遗留的问题。

用代码风格规范来解决代码风格的争议

关于代码风格的争吵在 code review 中是非常浪费时间的。一致的代码风格当然是非常重要的,但 code review 的时间并不该浪费在讨论圆括号该放在哪里。最好的做法是通过维护一份代码风格规范来避免这里的争吵。

总结

总的来说 code review 的过程中,双方都应该保持谦逊学习的态度,在 review 过程中,尽量避免僵持和冲突,一旦发生冲突时,leader 一定要快速找到平衡点,如果持续时间长,就会影响彼此的合作关系,代码问题可以慢慢去解决,和谐互助友好的团队关系才是更重要的。

结合前面有以下总结:

part1 - 好的 code review 需要做到以下

  • 更全面,从正确性到系统影响评估。
  • 注意语气,从给出建设性一觉到换位思考。
  • 及时完成审阅,从充分讨论到随机应变。
  • 加强交流,从面对面交流到灵活选择最高效的沟通方式。
  • 区分重点,从添加标记到利用工程化工具自动解决。
  • 对新人要更友好。
  • 尽量避免跨时区协作,必要时选择视频会议。

part2 - 良好的 code review 具体实施

  • 发起 code review 时,代码提交者应当提供一份变更列表
  • code review 完成后,以 LGTM 结尾(looks good to me)
  • 不用 review 风格格式问题,这些可以用 lint 工具去解决。团队规范可以改,给出有力的理由。
  • 收到 review 请求后要立即处理,自己太忙无法处理的,立即转给他人处理
  • 提出的问题不要太多,20-50 个以内
  • 提出问题的同时,给出 good code example
  • 评论中不要说“你”。可以用“我们”,或者干脆什么主语都不说直接说代码,或者用疑问请求语气代替命令式语气
  • 评论应该基于代码“原则”或规范,而不是自己的观点

part3 - 良好的 code review 具体实施

  • 注意上面的 5,因为想要把代码从 F 提升到 A,理论上是可行,当时会耗费作者和 reviewer 太多的精力和时间,可能还会在一来一回中造成不愉快。方法是,先从 F 提升到 C,下次他就会从 C 到 B 甚至是 A
  • 相同的问题,注明个 2-3 次即可,剩下的告诉作者修复其他类似问题
  • 不在变更范围的代码,有问题也不应该放在这次 review 里。如果是顺手可改的命名问题或极其容易修复的问题,可以作为 optional 提出来
  • Review 多个 300 行的变更体验好过 review 单个 600 行的变更
  • 看到代码里好的地方,让自己受益,可以适当表扬
  • 如果剩下的都是 变量命名和拼写错误,以及 optional 的修改点,也可以直接 approve,无需过于纠结。不拖延是要点;
  • author 和 reviewer 之间出现僵局怎么办?尽快的让步,或找管理者寻求帮助。僵持的时间越长,对合作关系的破坏越强,而这个关系远重要于代码质量 不要试图一次解决所有问题。