AI 日报

代码审查真的已经过时了吗?

  • By admin
  • Sep 19, 2023 - 2 min read



译者 | 刘汪洋

审校 | 重楼

反馈就像一块牛排 - 如果太生,没有人喜欢;但如果过熟,难以下咽。
(ChatGPT)

通过不断审查他人代码,你不仅可以提升自己的技能,对你的职业发展也有很大好处。不仅可以帮助别人成长,也能为你所在的公司创造价值。

在本文中,我们要探讨代码审查的好处,以及一些在审查过程中应遵循的原则,如与同事的互动等。

名词解释

代码审查:对作者代码分支的书面反馈过程。

Pull Request:GitHub 上用于展示新分支与主分支之间差异的术语,你可以在其中发表评论。

代码审查?是否已经过时?

如果你是一名敏捷开发者,你可能会怀疑代码审查的必要性,可能会有这样的观点:

  • 我们如果一直进行结对编程,就没有必要再做正式的代码审查了。因为这样就不会有一个“陌生人”在你已经完成所有工作并花费了大量时间之后,才来评价你的代码,而他对你的代码的上下文可能一无所知。
  • 代码的正确性已通过 TDD (测试驱动开发)方法进行了验证。
  • 语法和风格可以由 linter 自动检查。

这些考虑无疑是有道理的。

但:

代码审查本质上是一种团队成员之间和不同团队之间的知识共享机制

代码审查的好处

一旦你不再把代码审查视为负担或者无聊的任务,你会发现,代码审查能带来很多好处。

为了支持这个观点,我提出了一个用于在同事的 pull request 中提供反馈的框架。

W3H:即“为什么(why)、做什么(what)、何时(when)以及如何做(how)”

尽管我并非缩写词的狂热爱好者,但我还是创建了一个 W3H 的缩写词,旨在概括我们在接触新的代码时所需要考虑的关键问题。

为什么(why)

代码审查可能是由于你的组织内部的 CI/CD 流程强制推动的,因此,“为什么”的问题可以简单地回答为“因为我被指示要这样做”。

然而,有更有价值的问题值得我们思考:“为什么代码审查如此重要?”或者说,“为什么我应该主动去进行代码审查?”

首先,代码审查对审查者和代码作者来说都是一个提升自我技术水平的好机会。代码作者可以得到有效的反馈和建议,有助于其技术水平的提升。审查者则可以从中学习新的编程技巧和习语。

此外,作为开发者,你还可以:

  1. 更快地理解新的代码库。
  2. 有更多的机会进行团队内交流,尤其在团队成员分布在不同地理位置的情况下。
  3. 发现其他团队工作中可能存在的重复问题。
  4. 在代码库中推动最佳实践的实行。
  5. 提升你在其他工程师和管理者心中的专业形象和知识储备。
  6. 提高你的沟通技巧(我们将在“如何”一节中详细讨论)。
  7. 结识新朋友!

做什么(what)

我们如何定义“优秀的”代码?

在对别人的代码发表任何评价之前,你需要明确好代码的基本标准。

以下是一些通用的原则:

  1. 代码中不应存在 明显的 错误,例如变量名的拼写错误,或者不规范的代码缩进。当我审查一个我并不太熟悉的模块的 PR 时,我通常会从这些基础的检查开始。
  2. 无论代码完成的任务有多复杂,代码本身都应该是 结构清晰  易于理解
  3. 代码中不应存在严重的 性能问题,例如,一个可以通过一次循环读取完成的列表被多次读取。这在移动应用开发中尤其重要,因为这可能导致不必要的电池电量消耗。
  4. PR 只应该修改完成任务(新特性,错误修复,重构)所必需的 文件。这不仅可以减轻审查者的工作压力,而且如果出现了重大的生产问题,也会更容易找到问题并进行回滚。如果代码涉及多个方面,建议作者将其拆分为两个或更多的审查。
  5. 清晰理解更改的目标。PR 的描述应该清晰地说明更改的内容或者链接到包含详细信息的外部文档(例如 Jira 或 Trello 工单)。理解了更改的目标后,再检查更改是否满足了预设的要求。
  6. 如果 PR 是用来修复一个错误,修复方案还应该包含一组测试,这样可以针对特定的场景进行测试,并避免相同的错误在未来再次发生。

何时(when)

你应该在什么时候进行代码审查?

我通常每周安排两次,每次约 30 分钟的时间,审查其他团队的 PR,这些与我自己团队的工作并无直接关联。如果我团队有紧迫的项目截止日期,可能我会减少审查的时间;如果是工作相对清闲,并且有一些评论引发了大量的讨论,我可能会投入更多的时间。

在我刚开始接触 iOS 开发时,我花在代码审查上的时间和学习 Swift 和 iOS 的时间几乎一样多。这使我能够快速地熟悉新的代码库,语言习语,最佳实践,以及了解项目中的关键人物。

我通常在面对学习新的大型代码库时,会立即开始进行代码审查(这是作者有效学习和了解新代码库的一种方法)

如何(how)

编写大量的评论可能会让你感到枯燥乏味,因此你可能在表述上变得过于直白。书面交流与口头交流有所不同,我建议你遵循以下建议:

友善待人

这一点无需多说。通常情况下,当我向我不太熟悉的人写评论时,我会首先用一个简单的 "你好 " 打招呼。如果你发现有什么项目缺失,不要直接用 "这是不完整的" 来表示,你可以询问 "这里为什么会缺失?"

注意细节

尝试在关键的和不那么关键的修改之间找到平衡,这需要你的经验来指导。我们不希望因为一个小的空格问题而阻塞了一个重要功能的实现或者 bug 的解决!

询问作者的意见

如果你提出的改动不显著,或者你提出了重构的建议,那么最好在你的评论结尾处询问一下 "你怎么看?" 或者 "你对此有什么看法?"

有时需要权衡

有些项目比其他项目更为重要。尽量不要因为小的修改而坚持不懈,以免造成发布延迟。你可以与作者达成共识,以后再创建一个"打磨"分支。然而,我常说:

以后 == 永不(later == never)

不要做假设

你正在审查一个 PR 并发现了一个明显的错误,而这段代码的开发者的职称是"初级"。你可能会假设这个错误是由于他们的经验不足,或对一些基本概念的理解不够深入。也可能是他们匆忙完成的,或者在度过了一天的劳累后提交的。或者……可能有一些我们未能看到的、合理的原因在背后。如果有疑问,告诉作者: "可能是我漏掉了什么,但是……"

保持个人偏好的开放性

当你在代码审查中建议更改时,确保这些更改是对代码的实实在在的改进,而非审查者的个人偏好,这些偏好可能并不符合公司或行业的最佳实践。当我提出这样的更改时,我会说: "这是个人偏好的问题,但如果你愿意,可以试试<代码更改>"

赞美作者

“看起来很好(LGTM)” 可能会被解读为“我草率地审查了你的 PR”。对我来说,看到这个还好,但是,如果你真的认为代码写得很好,那就在 PR 中大胆地表扬。以下是值得赞扬的原因:引入了新的炫酷功能,或者进行了复杂但结构良好的重构。

如果 pull request 只包含一个简单的颜色更改或函数参数的添加,那么过度的赞美可能会被误认为是讽刺 。请选择适当的语言表达。

副作用

尽管你持有善意,或者在代码审查过程中为项目贡献良多,有时你的行动可能仍会引发他人的反感。他们可能按照你的建议进行修改,但可能并未对你的审查表示感激。

有些人可能会驳斥你的建议

你的建议可能错误,无效,或者触碰到了作者的自尊心,使他们认为修改代码就等于否定自己。这没有关系。然而,如果你认为某段代码可能会导致严重的问题或性能影响,你应该考虑邀请其他开发者一起讨论,例如在评论中@他们,引导他们参与讨论。更好的做法是,尝试直接通过电话或面对面的方式进行讨论。

不要期待你的付出一定会得到回报

你或许为许多代码审查和批准做出了贡献,帮助审查了他人的代码,但当你自己的代码需要审查的时候,不要期待你的新 pull request 一定会受到关注。

你可能会收获友情

去年,我为了更好地熟悉某个代码库,我对一个新接手的代码库进行了持续的代码审查。我与许多开发者进行了积极的讨论,最终,对仓库中的一些模块进行了改进。这是作者和我——这位新来的代码审查者的共同努力的结果!我结识了一些愿意改进他们代码、并始终对新建议和学习保持开放态度的优秀人才。季度结束时,我向他们请求正式反馈,收到了非常积极的回应。我们可以称这种关系为"联系",甚至我愿意称它为"友谊"。

结语

在这篇文章中,我们探讨了持续进行代码审查的诸多优点。 我推广的 W3H 方法,就是一种给他人的代码提供反馈的流程。

在我所工作的 Expedia Group™,我经常应用这种方法,公司也倡导并正式提出了"有意识的包容"这一价值观。

本文要点总结如下:

  1. 认识到审查他人代码的好处(比如建立联系,熟悉新的代码库,学习新的编码风格等)。
  2. 建立持续审查的习惯。
  3. 提供超越你团队范围的反馈。
  4. 避免过度假设。
  5. 不要期待每个人都会同等地关注你的 pull request。
  6. 保持友善。

你公司的代码审查流程是怎样的?你在工作中是否真正感受到代码审提高了代码质量?欢迎发表你的看法。

译者介绍

刘汪洋,51CTO社区编辑,昵称:明明如月,一个拥有 5 年开发经验的某大厂高级 Java 工程师,拥有多个主流技术博客平台博客专家称号。

原文标题:The Importance of Being a Code Reviewer,作者:Carlo Sales