Google 是如何做 Code Review 的?| 原力计划( 六 )


怎样写代码评论
概要

  • 礼貌
  • 解释你的观点.
  • 明确指出方向和问题 , 帮助开发人员去权衡作出决定.
  • 鼓励开发人员通过注释和精简代码来解决你的困惑而不是通过解释
礼貌
通常来说当你Code Review代码时保持礼貌和尊重能使开发人员更加清晰 , 得到更多帮助 。这样是为了保证你的代码评论仅仅针对的是code而不是针对开发人员 。你不必一直这么去做 , 但是当你的评论会让开发人员生气或者产生争执时有必要这么去做 。比如:
不好的例子: “你为什么会在这里使用线程 , 这样做难道会有任何好处?”
好的例子: "我并没有发现这个并发模块给程序带来了多少帮助 , 并且还增加了程序的复杂性 , 因此我认为这段代码最好是用单线程而不是多线程 。
解释清楚原因
从上面“好”的例子当中你能发现 , 这样有助于开发人员理解为什么你写了这些评注 。你不一定非得包含这些信息在你的评注里面 , 但是适当的多解释你的意图或者多给出一些提升代码质量的建议都是非常好的实践 。
给予指导
通常来说修复CL是开发人员的职责而不是评审人员的 。你不需要向开发人员提供详细的解决方案或者代码 。
但是这并不意味着评审员就不应该提供帮助 。你需要在指出问题和提供直接指导之间找到平衡 。指出问题并且帮助开发人员决策能够帮助开发人员学同事让code review变得更加简单 。这样也能产生更好的方案 , 因为开发人员比评审者更加了解代码 。
尽管这样 , 有时候直接给出指导 , 建议甚至是代码更有帮助 。code review的主要目的是尽可能得到最好的CL 。其次是提高开发人员的技能这样就能减少以后评审的次数 。
接受解释
与其要求让开发人员解释一段你看不懂的代码 , 其实更应该做的是让他们重写代码 , 让代码更清晰 。当一段代码不是太过于复杂的时候通过加一些注释偶尔也是一种不错的做法 。
解释仅仅是写在Code Review工具里的 , 不利于将来的代码阅读者 。只有极少数情况是可行的 , 比如你对你评审的需求不太熟悉 , 但是开发人员解释的是大多数人都知道的 。
处理Code Review中的反驳
有时开发者会在Code Review中反驳你 , 他们可能不同意你的意见 , 或者抱怨你太严格了 。
谁是谁非?
当开发者不同意你的建议时 , 首先花点思考下他们是否是对的 , 但通常而言你比他们更熟悉代码 , 所以可能在某个方面理解更深 。他们的争论有意义吗?从代码健康的角度来看他们的反驳有意义吗?如果是 , 让他们知道他们是对的 , 然后这个问题就解决了 。
然而 , 开发者不总是对的 , 这种情况下评审者应当进一步介绍为什么他们的建议是对的 。好的解释不仅得体现出对开发人员回复的理解 , 而且还要说明为什么要这么改 。
如果评审者认为他们的建议可以改善代码质量 , 并且他们认为带来的代码质量改进值得开发者做这些额外的工作 , 评审者就应该坚持自己的立场 。
不积跬步无以至千里 , 不积小流无以成江河
有时候要让开发者接受 , 你得花很多时间反复解释 , 但始终确保该有的礼貌 , 并让开发者知道在知道他们了什么 , 即便是你不同意 。
惹恼开发者
有时评审者会认为坚持让开发者做出改动 , 可能会惹恼开发者 。有时开发者确实会恼怒 , 但这种情况通常都很短暂 , 而且之后他们会感激你帮助他们提高了代码质量 。如果你在Code Review中很有礼貌 , 开发者根本不会被惹恼 , 这种担心是多余的 。通常 , 令惹恼开发者的是你写注解的方式 , 而不是你对代码质量的坚持 。
稍后解决
一种常见的反驳原因是开发者希望能尽快完成任务 。他们不想一轮又一轮地做Code Review , 然后就会说他们会在后续CL中处理这些问题 , 你只需要通过就行 。有些开发者做的很好 , 他们会立马提交后续CL处理这些问题 。然而 , 经验告诉我们原始CL通过之后拖的时间越久 , 就越不可能修复 。除非开发者在当前CL通过后立马修复 , 否则他们就不可能修复 。这并不是开发者不负责任 , 而是因为他们有好多工作要做 , 而修复工作也会因为工作压力而被遗忘 。所以最好坚持让开发者现在就在CL中处理掉这些问题 , “留着以后清理”是一种不可取的方式 。


推荐阅读