如何进行有效的代码审查
在软件开发过程中,代码审查流程是确保代码质量、促进协作和团队成员知识共享的关键检查点。尽管非常重要,但许多工程师对如何进行有效的审查缺乏清晰的思维导图。这是我为帮助代码审查和审查人员改进而做的尝试。
第一个审查问题:意图是什么?
首先关注 git 提交的标题和信息。您理解描述吗?仅从描述来看,建议的修改有意义吗?想法是否清晰?扪心自问,您是否能想出不做改动的理由,或者是否有明显更好的替代方案。
如果描述对您没有意义,请立即将其作为您的初步反馈意见与他人分享。如果有关意图的描述无法理解,就不要浪费时间审查代码实现的细节。这可能是因为提交的内容还只是一个草稿,唯一的(也是最直接的)反馈应该是要求提交者澄清他们的意图。
接下来要检查的是代码是否与描述相符。例如,如果修改是为了修复一个错误,那么就不应该包含增加新功能的额外代码。有时,当您查看代码时,90% 的改动都是合理的,与 git 提交信息中的描述完全一致,但可能有一些代码行的改动您不理解。请询问提交者。也许是错误地包含了这些代码,或者是出于某种不明显的原因。应该在代码旁边加上注释,或修改变更说明。
下一步:实施效果好吗?
假设描述和变更一致且有意义,那么请将重点转移到实现细节上,并评估问题是如何解决的。作为审核员,您应该问自己是否有实现相同结果的替代方法?这些替代方法会带来明显的优势,还是当前的建议是最合理的?
如果代码改动看起来大部分都不错,而且您预计在下一次修订中不会进行全面修改,那么就可以对代码中的细节提出反馈意见。查找潜在的逻辑错误、与编码规范的偏差、反模式、拼写错误,甚至暗示任何您可能预料到的安全问题或性能问题。仔细检查吸引你眼球的每一个方面,并尽可能提出改进建议。
如果您发现自己几乎对每一行更改的代码都发表了评论,请考虑中止审核,并要求提交者在完成全面审核之前处理第一组评论。
注意有些人可能会从字面上理解每一条评论。在指出可以改进的地方时,切记使用 “一般来说,最好…… “或 “如果您有时间,请…… “这样的短语来弱化一般性反馈,并明确在您批准修改之前,哪些地方确实需要解决,哪些地方您只是建议进行质量改进,而没有坚持要求。还要提防那些机械处理意见的提交者。与其以代码片段的形式给出自己建议的更好的实现,不如直接要求提交者 “请重新考虑 X,这样它就能一直做 Y 而不做 Z”,这就迫使提交者自己编写更好的版本,并在这一过程中训练他们的脑力。谁知道呢,在对你指出的问题进行思考后,提交者甚至可能写出比你最初想到的更好的修复方案。
吹毛求疵不仅可以,而且非常重要
有些人可能会对在代码评审中吹毛求疵感到犹豫,因为吹毛求疵并不是文明人在日常交流中经常做的事情。然而,吹毛求疵是代码评审和软件工程中不可或缺的一部分。计算机会从字面上读取每一个点和逗号,而这些点和逗号必须完全正确。无论从短期还是长期来看,代码审查的整个理念都是最大限度地提高质量和减少出错的可能性。因此,在代码审查中,请尽可能地吹毛求疵。
反馈的数量应该是最大的,但批准的标准不必是最高的。对最低质量的要求应适合开发团队的技术水平。是的,随着时间的推移,质量标准可以随着开发团队对最佳实践的集体学习而提高,但如果质量标准从一开始就过高,就会打击开发人员提交新作品的积极性。这样就无法实现学习的迭代循环。
质量管理很难,但确保可接受的质量是代码审查流程的核心。不过,一般来说,如果你对什么是合理的要求有疑问,那就偏向于要求更高的质量,而不是满足于你觉得不合格的东西。从长远来看,您更有可能为做出这样的决定而感到高兴。
请记住,人们满足于低质量的主要原因是,以最高质量水平做事需要付出更多的努力,而有些人是懒惰的。但是,如果不断推动人们以高质量做事,那么正确做事很快就会变得不费吹灰之力。
持续集成和测试节省每个人的时间
为了支持人工审核流程,所有软件项目都应该有一个 CI,它可以对每次代码更改进行自动测试,并帮助检测至少所有容易检测到的回归。CI 失败的代码提交通常不会吸引很多审核人员,因此提交者应尽快自行审核并修复所有 CI 问题。
与此相关,所有改变程序行为的高质量代码提交也应更新和扩展相关的测试代码。举例来说,如果软件项目有单元测试,而提交者在提交新代码的同时却没有提交新的单元测试,那么审核员就应该要求提交者编写缺失的测试。这种反馈甚至可以是自动的–一个好的 CI 系统可以检测到测试覆盖率的下降。
清晰准确地沟通
在评审过程中,提交者和评审者都必须清晰准确地进行沟通。
从审稿人的角度来说,这意味着审稿人应该清楚地知道,哪些东西是需要提交者提供的,哪些东西只是很好提供的。如果审稿人认为整个方法都不好,他们就应该坦率地从一开始就拒绝提交。
从提交者方面来说,初次提交的代码应尽可能完整。如果提交代码供审核(例如:Pull Request 或 Merge Request),但代码还没有最终内容,提交者应明确说明这一点,并在其 git 提交或审核标题前加上 WIP:
,以表示代码正在开发中。
根据我的经验,代码提交和评审停滞不前的典型原因就是沟通不畅。提交的代码可能是完全正确的,但缺乏通俗易懂的英语部分,从而导致沟通不畅。通常情况下,审阅者也会推迟深入研究他们一看就不明白的提交内容,因为要对不清楚的代码提交内容进行侦查工作,会让他们感到不知所措。因此,确保提交的代码及时得到审核的最佳方法就是清晰地沟通。
避免噪音,最大限度地增加 “信号”
不用说,提交者和审核者都应该知道如何正确使用审核工具。例如,GitHub 和 GitLab 都有 “开始审阅 “和 “完成审阅 “功能,允许审阅者撰写多条评论,而不会向提交者发送多封邮件。按下 “完成审核 “按钮将触发一封包含所有评论的邮件提交。大多数系统都有请求审核和重新请求审核的按钮,提交者应使用这些按钮来清楚地传达审核反馈意见。
当提交者重新提交代码以供审核时,他们应该始终使用 git push --force
来进行操作。审稿人总是带着 “这可以合并吗?”这样的问题来看待提交的代码,而他们看到的提交应该是最终打磨过的代码。不应该有任何 WIP
提交或多个中间 git 提交–审稿人对提交者如何获得正确的最终结果并不感兴趣。只有最终版本才是最重要的。
尊重他人的时间,正确安排优先次序
评审需要时间。提交者应格外小心,不要浪费审稿人的时间。同样,审稿人也应尽快审稿,或至少以简短评论的形式分享他们的初步印象。
如果审稿人时间不够,他们应该优先重新审阅他们已经给出过反馈的投稿。已经对某件事情有所了解的人应该继续关注,因为这对他们来说是最有效率的。团队中的其他人则更有可能审阅没有任何评论的代码提交。
同样,提交者应优先考虑回复审核反馈,并尽快更新和完善现有的提交内容。审核员更有可能在几天前重新审核并批准他们已经看过的提交,而不是已经拖了几周或几个月的提交。
尊重原始代码,让提交者感受到所有权
代码提交和审查过程是新提交者开始参与软件项目的过程。评审人员应牢记这一点,不要急于抢夺代码,而是要多花一点精力,在代码基础和质量标准方面对提交者进行指导。审核员有时可能会感到沮丧,但这不能成为不良行为的借口。
有时,审核员可能会想自己解决提交中的问题,而不是给原始提交者一个机会来做最后的润色。审稿人必须抵制这种做法,因为这会扼杀原始提交者的主人翁意识。在开放源代码项目中,攫取他人提交的内容并将其提交给自己也可能构成侵犯版权,而且这肯定不会鼓励提交者继续提交更多内容。审阅者最多只需重新提交,仅此而已。
如果审查过程中来回折腾的次数太多,那么作为一种折中方案,审查员可以将提交的内容按原样合并,然后立即在提交的基础上进行自己的修改。
无论是在开源项目还是公司内部工作中,代码被合并的成就感都是非常宝贵的,尤其是如果这是该人对某个项目做出的第一个被接受的贡献。不要剥夺代码提交者的荣誉,让他们自己去争取,在他们的个人档案中留下自己的 git 学分和紫色的 GitHub 合并徽章。
可以有很多审核员,但绝不能超过一个代码提交者
代码提交的基本原则是保持单一的代码提交者。虽然可能会有多人审核和发表评论,但提交代码(以及随后的改进修订)的人绝不能超过一人。只有一个作者才能确保有一个明确的所有者,由他迭代提交的代码并决定如何处理所有反馈。如果使用的是 git 分支,那么只有这个人才能修改提交和强制推送分支,直到最终版本完成。
如果提交的是邮件列表中的补丁,则不可能出现一封邮件有多个提交者的情况,作者和所有权也不是问题。在 GitHub 或 GitLab 上,这个问题可能会出现,因为提交的是一个 git 分支,而该分支的权限可能允许多人向其推送。然而,多人推送同一个拉取请求则是完全错误的。
有些项目故意滥用 git 拉取请求,让多个作者在上面推送 git 提交,并在 “审阅评论 “部分讨论协作情况。要让一群人在主线分支上线前一起在 git 中编写代码,正确的方法是建立一个特性分支。GitHub 拉取请求和 GitLab 合并请求都允许提交者选择提交的目标分支。在特性分支模式下,每个合作者都会向特性分支提交各自的拉取请求,一旦特性完成,特性所有者就有责任将特性分支纳入 git 项目的主线分支。
有时可能会出现意见分歧或懒惰的情况,提交者拒绝妥善处理所有反馈。在这种情况下,接收者/审阅者要么完全驳回提交,要么按原样接受(合并到目标分支),其他人可以通过跟进提交的形式继续工作,进一步改进。
也可能出现这样的情况:提交的想法很好,但实施起来却很糟糕,令人无法接受。提交者被要求改进实现,但提交者可能做不到。在这种情况下,其他人都不应该重写同样的提交内容,因为这会模糊作者和所有权的界限。相反,其他人如果有更好的实现,只需打开一个新的拉取请求,或以自己的名义提交电子邮件,并附上自己的版本。之后,将由审稿人决定哪些提交被接受,哪些被拒绝。
代码审查是学习的机会–对代码提交者和审查者都是如此
请记住,审核员不一定是优秀的开发人员。任何人都可以成为审核员,无论他们是资深还是初级软件开发人员。阅读别人编写的代码是接触各种编码风格和不同开发人员解决编码问题的各种方法的好方法。审阅代码对审阅者来说是一个学习的机会,因此审阅过程最终会让提交者和审阅者在将来写出更好的代码。
如果有机会进行代码审查,即使你不觉得自己是某个领域的权威,也要去做。您仍然可以通过提供某些方面的反馈意见做出有价值的贡献,而将最终的审批决定权留给领域专家。
记住要清晰地沟通–代码记录得越好,反馈解释得越清楚,提交者和审核者就能从中学到更多。
如果您对代码审查的最佳做法有其他建议,请在下面添加评论!
本文文字及图片出自 How to conduct an effective code review