同行代码审查(Peer Code Review)实战经验
我有时候会听到我们的团队成员这样议论:
"项目的Code review 只是浪费时间。"
"我没有时间做Code review。"
"我的发布时间延迟了,因为我的同事还没有完成我代码的Code review。"
"你相信我的同事居然要求我对我的代码做修改吗?请跟他们说代码中的一些联系会被打断——如果在我原来代码的基础之上做修改的话。"
(LCTT 译注:Code Review中文可以翻译成代码审查,一般由开发待review的代码的成员以外的团队成员来进行这样的工作。由于是专业术语,没有将Code review用中文代替。)
为什么要做Code review?
每个专业软件开发者都有一个重要的目标:持续的提高他们的工作质量。即使你团队中都是一些优秀的程序员,但是你依然不能将你自己与一个有能力的自由职业者区分开来,除非你从团队的角度来工作。Code review是团队工作的一个重要的方面。尤其是:
代码复查者(reviewer)能从他们的角度来发现问题并且提出更好的解决方案。
确保至少你团队的另一个其他成员熟悉你的代码,通过给新员工看有经验的开发者的代码能够某种程度上提高他们的水平。
公开reviewer和被复查者的想法和经验能够促进团队间的知识的分享。
能够鼓励开发者将他们的工作进行的更彻底,因为他们知道他们的代码将被其他的人阅读。
在review的过程中的注意点
但是,由于Code review的时间有限,上面所说的目标未必能全部达到。就算只是想要打一个补丁,都要确保意图是正确的。如果只是将变量名改成骆驼拼写法(camelCase),那不算是code review。在开发过程中进行结对编程是有益处的,它能够使两个人得到公平的锻炼。你能够在code review上花许多时间,并且仍然能够比在结对编程中使用更少的时间。
我的感受是,在项目开发的过程中,25%的时间应该花费在code review上。也就是说,如果开发者用两天的时间来开发一个东西,那么复查者应该使用至少四个小时来审查。
当然,只要你的review结果准确的话,具体花了多少时间就显得不是那么的重要。重要的是,你能够理解你看的那些代码。这里的理解并不是指你看懂了这些代码书写的语法,而是你要知道这段代码在整个庞大的应用程序、组件或者库中起着什么样的作用。如果你不理解每一行代码的作用,那么换句话说,你的code review就是没有价值的。这就是为什么好的code review不能很快完成的原因。需要时间来探讨各种各样的代码路径,让它们触发一个特定的函数,来确保第三方的API得到了正确的使用(包括一些边缘测试)。
为了查阅你所审查的代码的缺陷或者是其他问题,你应该确保:
-
所有必要的测试都已经被包含进去。
-
合理的设计文档已经被编写。
再熟练的开发者也不是每次都会记得在他们对代码改动的时候把测试程序和文档更新上去。来自reviewer的一个提醒能够使得测试用例和开发文档不会一直忘了更新。
避免code review负担太大
如果你的团队没有强制性的code review,当你的code review记录停留在无法管理的节点上时会很危险。如果你已经两周没有进行code review了,你可以花几天的时间来跟上项目的进度。这意味着你自己的开发工作会被阻断,当你想要处理之前遗留下来的code review的时候。这也会使得你很难再确保code review的质量,因为合理的code review需要长期认真的努力,最终会很难持续几天都保持这样的状态。
由于这个原因,开发者应当每天都完成他们的review任务。一种好办法就是将code review作为你每天的第一件事。在你开始自己的开发工作之前完成所有的code review工作,能够使你从头到尾都集中注意力。有些人可能更喜欢在午休前或午休后或者在傍晚下班前做review。无论你在哪个时间做,都要将code review看作你的工作之一并且不能分心,你要避免:
-
没有足够的时间来处理你的review任务。
-
由于你的code review工作没有做完导致版本的推迟发布。
-
提交不再相关的review,由于代码在你review期间已经改动太大。
-
因为你要在最后一分钟完成他们,以至于review质量太差。
书写易于review的代码
有时候review没有按时完成并不都是因为reviewer。如果我的同事使用一周时间在一个大工程中添加了一些乱七八糟的代码,且他们提交的补丁实在是太难以阅读。在一段代码中有太多的东西要浏览。这样会让人难以理解它的作用,自然会拖慢review的进度。
为什么将你的工作划分成一些易于管理的片段很重要有很多原因。我们使用scrum方法论(一种软件开发过程方法),因此对我们来说一个合理的单元就是一个story。通过努力将我们的工作使用story组织起来,并且只是将review提交到我们正在工作的story上,这样,我们写的代码就会更加易于review。你们也可以使用其他的软件开发方法,但是目的是一样的。
书写易于review的代码还有其他先决条件。如果要做一些复杂的架构决策,应该让reviewer事先知道并参与讨论。这会让他们之后review你们的代码更加容易,因为他们知道你们正在试图实现什么功能并且知道你们打算如何来实现。这也避免了开发者需要在reviewer提了一个不同的或者更好的解决方案后大片的重写代码。
项目需要应当在设计文档中详细的描述。这对于一个项目新成员想要快速上手并且理解现有的代码来说非常重要。这从长远角度对于一个reviewer来说也非常有好处。单元测试也有助于reviewer知道一些组件是怎么使用的。
如果你在你的补丁中包含的第三方的代码,记得单独的提交它。当jQuery的9000行代码被插入到了项目代码的中间,毫无疑问会造成难以阅读。
创建易读的review代码的另一个非常重要的措施是添加相应的注释代码。这就要求你事先自己做一下review并且在一些你认为会帮助reviewer进行review的地方加上相应的注释。我发现加上注释相对于你来说往往只需要很短的时间(通常是几分钟),但是对于review来说会节约很多的时间。当然,代码注释还有其他相似的好处,应该在合理的地方使用,但往往对code review来说更重要。事实上,有研究表明,开发者在重读并注释他们代码的过程中,通常会发现很多问题。
代码大范围重构的情况
有时候,有必要重构一段代码使其能够作用于多个其他组件。若是一个大型的应用要这样做,会花费几天甚至是更多的时间,结果是生成一个诺大的补丁包。在这种情况下,进行一个标准的code review可能是不切实际的。
最好的方法是增量重构你的代码。找出合理范围内的一部分改变,以此为基础来重构。一旦修改和review完成,进入第二个增量。以此类推,直到整个重构完成。这种方法可能不是在所有的情况下都可行,但是尽管如此,也能避免在重构时出现大量的单片补丁。开发者使用这种方式重构可能会花去更多的时间,但这也使得代码质量更高并且之后的review会更简单。
如果实在是没有条件去通过增量方式重构代码(有人可能会说之前的代码书写并组织的是多么的好),一种解决方案是在重构时进行结对编程来代替code review。
解决团队成员之间的纠纷
你的团队中都是一些有能力的专家,在一些案例中,完全有可能因为对一个具体编码问题的意见的不同而产生争论。作为一个开发者,应该保持一个开发的头脑并且时刻准备着妥协,当你的reviewer更想要另一种解决方法时。不要对你的代码持有专有的态度,也不要自己持有审查的意见。因为有人会觉得你应该将一些重复的代码写入一个能够复用的函数中去,这并不意味着这是你的问题。
作为一个reviewer,要灵活。在提出修改建议之前,考虑你的建议是否真的更好或者只是无关紧要。如果你把力气和注意力花在那些原来的代码会明确需要改进的地方会更加成功。你应该说"它或许值得考虑..."或者"一些人建议..."而不是”我的宠物都能写一个比这个更加有效的排序方法"。
如果你真的决定不了,那就询问另一个你及你所审查的人都尊敬的开发者来听一下你意见并给出建议。
via: http://blog.salsitasoft.com/practical-lessons-in-peer-code-review/