代码审查

开源代码由具有不同背景、兴趣和目标的社区共同维护。因此,提供清晰、文档完善且可维护的代码和流程至关重要。代码审查是一种引导过程,用于集体发现潜在问题、提高代码质量,并让贡献者和审查者了解代码库及其前提假设。这也是确保有多人能够共同维护相关代码的一种机制。我们鼓励贡献者在请求审查前将代码打磨至可审查状态。这对准提交者尤为重要,因为提交者不仅需要参与编写代码,还需要参与代码审查。

本文档是开源代码审查的现行指南。请同时抽空阅读TVM社区准则了解一般开发流程。

建立信任

首先,我们正在建立一个基于信任的社区,这需要时间和精力来建设和维护。我们希望社区成员能以建设性的方式合作,并运用常识共同协作。尽管我们各自拥有不同的背景、兴趣和目标,但我们必须共同努力寻找适合更广泛社区的解决方案。基于信任的协作也是Apache方式的核心原则之一,是在社区发展和推动成员担任正式角色时需要考虑的重要因素。

社区参与

欢迎大家对PR发表评论。我们鼓励提交者在合并包含重大架构变更的PR之前等待一段时间(例如三天)。这样做的目的是让人们有时间发表意见,表达参与审查的兴趣。

在这里,记住我们来自不同背景非常重要。例如,一些社区成员在不同时区工作,只在工作时间参与开源项目,或者可能正在旅行或生活中遇到其他事务。大型项目协作的关键在于确保集体理解,避免形成个人瓶颈。虽然留出时间参与代码审查很重要,但我们也不能因为等待所有评审者而阻碍所有变更。请记住,帮助他人成功合并PR是鼓励更广泛参与的好方法,特别是对那些自愿贡献时间的贡献者来说。

这其中一部分在于信任并与共同维护者沟通,确保如果未来需要应用变更,PR作者会履行他们的承诺。提交者有责任听取所有反馈,无论是来自PMC成员还是新贡献者,并考虑需要采取哪些行动。

仔细阅读代码

有时我们可能会快速浏览代码,只关注代码的某些选择性方面。这类评论通常很有帮助,应该受到社区的欢迎。然而,它们只是进行代码审查的一部分,应该作为更全面反馈的一部分。一次良好且细致的代码审查需要投入大量时间,有时甚至可能比实际贡献的编写时间更长。

例如,当你的PR(拉取请求)只收到针对次要方面的高度严苛反馈时,这很少会让人感到愉快,如果在评审过程中你的时间和努力没有得到相应的回报,可能会令人沮丧。无论是作为贡献者还是提交者,实践同理心都很重要,这可以帮助你成为更有效的代码评审者和贡献者。

我们希望所有代码提交者在签署确认前仔细阅读并理解代码。当提交者按下合并按钮时,这涉及高度的信任。同时,我们也承认有时问题会遗漏,在这种情况下,合并者有责任确保采取正确的后续行动。

尊重他人

  • 致所有发表评论的朋友:提出建设性意见将有助于新贡献者及时提交PR,也能帮助我们更好地欢迎新成员加入社区。

  • 致作者:审阅者需要投入大量时间阅读代码,细致的审阅可能和从头编写代码一样耗时。请礼貌回应审阅意见,并通过在未来帮助审阅他人变更来回报审阅工作。

最重要的是专注于进行建设性的对话,并在作为评审者互动时尽量以最好的意图去理解对方。如果流程中有任何问题,考虑与其他贡献者面对面交流,讨论如何改进流程或沟通。

关于代码质量需要考虑的因素

高质量的代码对项目的长期成功至关重要。在代码审查过程中需要考虑许多代码质量因素:

  • F0: 整体架构。这包括公共模块的定义、关键数据结构和公共接口。良好的架构选择对项目的长期成功至关重要。

  • F1: 架构一致性。实现新功能通常有多种方式。我们必须确保新功能与之前的整体架构选择保持一致,并与现有代码良好交互。每个新功能都会增加项目的复杂性,而一致的设计理想情况下可以最小化新功能带来的复杂性增长,从长远来看更易于维护代码。

  • F2: 代码健壮性和测试覆盖率。确保代码在所有可能的设置(平台)下正确运行,确保新功能的测试覆盖率。面向用户的错误需提供清晰的错误信息。

  • F3: 用户界面API文档:必须提供面向用户的公共API和关键模块接口的文档。这包括出现在公共接口中的API和数据结构(即include/tvm和面向用户的Python API)。我们通常鼓励对代码进行良好文档化,并为在多处使用的内部API提供某种形式的文档,另见F4。

  • F4: 代码可读性。可读性涉及多个方面:具有指导性和一致性的函数命名,清晰的整体流程实现,对复杂代码逻辑和内部函数的描述性注释。可读性高的代码更易于维护。

架构设计和一致性是最重要的因素,因为它们很可能带来最多的长期技术债务。 因此,提交者在合并代码前应最慎重地考虑这些因素。

代码贡献需要包含测试覆盖率和API文档。

代码可读性相比其他方面是一个相对主观的问题。 不同的人对如何编写最佳代码有不同的看法。评审者应提出建设性和可操作的评论。 同时,代码审查不应被用作要求他人完全按照你的方式编写代码的手段。 反过来,你也应该考虑到你容易理解或认为可接受的内容,可能并不适用于更广泛的社区或其他成员。 根据贡献内容和范围以及贡献者的背景,运用你的判断力来决定什么是合适的。

我们在编写代码时遵循通用的代码指南与技巧。风格指南有助于确保代码在原作者离开很久之后仍能被他人阅读和维护。风格指南不仅仅涉及代码格式——它们还包括正确记录代码的方式、变量命名以及其他自动化格式化工具无法强制执行的约定。

共识构建

在代码审查过程中可能会出现意见分歧。我们鼓励相关人员之间建立共识。在开源项目中,我们共同努力并相互建立信任。开源的本质意味着有时我们会在不太重要的问题上做出妥协,以稳步推进项目并欢迎更广泛的社区参与。遗憾的是,妥协意味着有时世界不会完全如我们所愿,即使对社区领导者来说也是如此。

  • 保持文明,通过基于技术的建设性对话达成共识。

  • 拥有该领域权限的提交者可以作为引导者,通过综合考虑所有讨论内容来推动讨论进程,并提出下一步的解决方案建议。

  • 由于提交者(导师)承担了很大的信任责任,他们应在签署前仔细审阅PR。此外,合并者也应负责跟进,以防合并引发问题。

一致性

最后要说明的是,我们都是凡人,很难始终保持完美的一致性。如果贡献者认为您没有以一致的方式应用这些准则,倾听和理解大家的意见非常重要。随着社区的发展,我们将不断迭代流程和准则。我们的目标是努力做到一致和客观,但遗憾的是我们都是有缺陷的凡人,都需要不断调整和学习。

Additional Recommendations

关于API和数据结构的深思熟虑

一个最小化且稳定的API对项目的生命周期至关重要。优秀的API能带来巨大改变。务必仔细考虑所有方面,包括命名、参数定义和行为。

在代码审查时,应尽可能更多地关注提议的API设计。请记住,改进代码实现相对容易,但API一旦被接受就极难更改。对于跨模块共享的数据结构(如AST),我们应以同样的方式对待。如有不确定之处,应在提交前与更多开发者展开讨论。

以下是设计API时的一些有用原则:

  • 如果功能有重叠,应与现有知名包的API保持一致。例如,张量运算API应始终与numpy API保持一致。

  • 与同一项目中的现有API保持一致。 例如,我们应该在所有优化过程中使用相同的参数顺序, 这样在使用时就不会出现"意外"。

  • 考虑API未来是否会发生变化。 例如,随着我们在构建中添加更多优化选项,我们将拥有诸如循环展开和设备放置策略等更多选项。 我们可以将优化参数打包到一个构建配置对象中。这样,即使功能不断丰富,构建API也能保持长期稳定。

  • 编写文档。对于API来说文档是必需的,有时编写文档还能帮助我们进一步思考设计,以及是否需要添加更多说明。

  • 最小化。考虑用户需要使用多少行代码来调用API。在可能的情况下减少抽象层级。

最小化依赖

在引入依赖项时务必谨慎。虽然重用代码和避免重复造轮子很重要,但依赖项会增加用户在部署时的负担。一个好的设计原则是:只有当用户实际使用某项功能时,才应该引入相应的依赖。

简洁实现

这里应用了一些基本原则:优先使用向量化数组代码而非循环,利用现有API来解决问题。

代码审查中的文档课程

当你发现有一些可以总结的常见或重复出现的经验教训时, 请将其添加到代码指南与技巧中。 在请求变更时参考这份指南文档总是有益的, 这样经验教训就可以分享给整个社区。

学习其他代码审查

可能会有多位评审者同时审查同一批更改。很多时候,其他评审者可能会发现你未注意到的问题。尽可能从他人的代码审查中学习,并将这些经验教训记录下来。

显式批准和请求变更

贡献者和代码所有者可以请求多位评审者进行代码审查。 当您的意见在代码审查中得到解决时,请记得批准更改。 为此,请在拉取请求中点击更改选项卡,然后选择批准, 或对代码发表评论并点击请求更改。 如果部分评审者未及时回应(例如一周内)且现有评审已足够, 代码所有者可以视情况决定是否合并代码。

审阅者

审阅者应努力及时对请求他们审阅的拉取请求提供反馈。代码审查是项目健康的重要组成部分,应被视为贡献者的常规责任。在这方面,自动化工具可以提供帮助,因为在一定时间内没有活动的PR会收到机器人评论,提醒相关方。