分享

高级工程师如何审查代码

 技术的游戏 2023-05-23 发布于广东

当审查拉取请求时,我看到了完成要求并立即获得批准的漂亮代码。

我也见过长轮来回,要求改变。令人困惑或过于努力的代码。

这是我作为高级工程师和技术主管在代码审查中寻找的东西。我也尝试让我自己的代码符合这些标准。

尺寸

乍一看,我对拉取请求的最初反应是:

“改变了多少?”

有两个原因:

  1. 第一个是自私的。我需要知道 PR 审核需要 10 分钟还是一个小时。如果你的 PR 真的很大,我可能需要稍后再审核。如果您想要快速审查,请瞄准更短的 PR。

  2. 第二个是思考变革管理。当您对代码库进行重大更改时,出错的机会就会增加。通常,对于大型拉取请求,您可以通过多种方式分阶段单独发布部分代码。这些类型的增量更改更可取。

教训是让你的拉取请求尽可能小。根据需要,创建从初始 PR 分支出来的后续 PR,以允许按顺序发布代码。

测试

接下来,我查看拉取请求是否包含测试。测试应该清楚地展示新代码的行为和预期结果。

我将尝试考虑可能尚未测试的边缘案例并对其发表评论。作为 CI/CD 的一部分,我鼓励我工作的团队针对存储库运行代码覆盖率,以确保所有新代码都经过测试。

测试帮助我作为审阅者理解代码。在我看来,它们在合并任何代码之前也是必不可少的。

结构

现在我已准备好查看新的逻辑代码,我将从评估解决方案的整体结构开始。

因为我只是查看了测试,所以我对代码如何协同工作来解决问题有一个想法。我将浏览已承诺回答一些问题的文件、类、函数等:

  • 这个新代码的位置有意义吗?

  • 它使用逻辑类结构吗?它应该从某个现有类继承还是以某种方式共享逻辑?

  • 函数和类的参数是否合乎逻辑且命名合理?

  • 函数返回什么?那有意义吗?

  • 我们可以简化实现的结构吗?

逻辑

解决了这些大问题后,我将深入研究代码的实际内容。

在我工作过的大多数团队中,我们使用静态分析工具来自动检查代码质量。这些工具可以解决大部分明显的错误、安全漏洞和低效问题。有一个自动的第一次通过并知道你正在审查的任何 PR 至少满足一个基本阈值是很有用的。

当我评估一个函数或类时,我会按以下顺序寻找一些关键的东西:

  • 是否有意义?— 如果不是,那就是危险信号。这意味着代码比它需要的更复杂。

  • 浪费吗?— 培养算法复杂性的直觉是一项困难但有用的技能。通常,这意味着一些简单的事情,比如使用字典而不是列表,只迭代一次并记住结果,或类似的事情。

  • 它可以更简单/更清洁吗?— 看到一条不必要的线路或一种可以更简单地做某事的方法并不少见。在大多数情况下,将其归因于开发人员连续数小时处理同一问题 — 无法发现错误,因为他们已经在问题中待了很长时间。

命名

新代码文档的第一行是很好的命名。

我所有的代码审查都包括我思考变量、函数和类名称是否有意义的部分。我们可以使用更短的名称吗?所有名称是否都清楚地传达了它们是什么或它们做什么?

我的一个规则是没有缩写。虽然他们现在看起来很聪明,但随着时间的推移,他们会变得站不住脚。所有的名字都需要拼写出来并清楚它们的意思。

命名是软件开发中最困难的部分之一。

文档

最后,我检查代码是否有适当的文档。

我并不是说开发人员需要编写有关代码及其作用的段落。只是每一段代码都应该有一个简单的描述它是如何工作的。

例如,在 Python 中,我们使用文档字符串来做到这一点。每个新函数或类都需要有一个文档字符串。在 JavaScript 中,出于同样的目的,我在函数定义之前使用了内联注释。

我还鼓励团队为他们的参数和返回值使用类型。类型注释是我寻找的另一份文档。

适当的文档还意味着在您更改内容时更新现有文档。在代码审查中,我试图提醒开发人员在过时时更新文档字符串和注释。

另一大块是外部文档。如果更改影响外部 API,则需要使用新信息更新 API 文档。

未来 PR 的清单

希望本文为您提供了寻找内容的良好基础。当您提交自己的代码以供审查时,您可以将其用作清单,以确保在请求他人审查之前您已涵盖所有内容。

当您进行自己的审查时,请随意使用它作为分析其他人代码的指南。

如果你喜欢我的文章,点赞,关注,转发!

    转藏 分享 献花(0

    0条评论

    发表

    请遵守用户 评论公约

    类似文章 更多