跳到主要内容
版本:0.21.0

代码审查

开源代码是由背景、兴趣、目标多样的社区成员共同维护的。因此,提供清晰、文档齐全、易维护的代码和流程非常重要。代码评审是一种协作过程,旨在集体发现潜在问题、提升代码质量,同时让贡献者与评审者深入了解代码库及其设计假设。它也是保证多个开发者可以共同维护某一部分代码的机制之一。我们鼓励贡献者在请求评审之前将代码打磨到可评审的状态。对于候选提交者(committer)来说尤其重要,因为提交者不仅要写代码,还要参与评审工作。

本文档是关于开源代码评审的活动文档。建议同时阅读 TVM 社区指南 了解通用的开发流程。

建立信任

首先,我们正在建立一个基于信任的社区,而信任的建立与维护需要时间和努力。我们期望社区成员本着建设性的态度协作,秉持常识,寻求对更大社区有益的解决方案。虽然我们背景和目标不同,但基于信任的协作是 Apache 精神的核心,也是推动社区成长和成员晋升的重要基础。

社区参与(Community Participation)

欢迎每个人对 PR 发表评论。我们鼓励提交者在合并涉及重大架构变更的 PR 前等待一段时间(例如三天)。这样做的目的是为了给社区成员留出时间表达观点、表示参与意愿,并有机会进行审阅。

在这里,牢记我们每个人都来自不同的背景是非常重要的。例如,一些社区成员可能处于不同的时区,只在工作时间参与开源项目,或者正好在旅行或处理其他事务。在一个大型项目中,确保集体理解是很重要的一部分,这样就不会让某一个人变成瓶颈。虽然给予代码审阅参与一定的时间很重要,但我们也不能因为等待所有审阅者而阻碍所有变更的推进。请记住,帮助他人成功合并 PR 是鼓励更广泛社区参与的好方法,尤其是对那些自愿贡献时间的开发者而言。

这其中也包括信任与维护者之间的沟通:即便某些修改需要在后续补充,我们也相信 PR 的作者会兑现他们的承诺。此外,提交者有责任倾听来自 PMC 成员或新贡献者的所有反馈,并据此考虑应采取的行动。

认真阅读代码

有时我们快速浏览代码,只关注了某些方面,这类评论虽然有价值,但只是完整代码评审的一部分。高质量的代码评审是一项耗时的工作,有时所花的时间甚至超过写代码本身。

例如,如果一个 PR 只收到对细节的高强度批评,而没有回应贡献者的整体努力,这种反馈是令人沮丧的。因此,无论是作为提交者还是评审者,都要有同理心,这将有助于你成为更有效的代码参与者。

我们要求提交者在签署通过前,认真阅读并理解代码。按下合并按钮本质上是一种信任行为。如果之后出现问题,合并者应负责采取适当的后续措施。

保持尊重

  • 对所有评论者:提出建设性的评论有助于新贡献者更快地推进 PR,同时也能帮助我们欢迎更多社区新成员。
  • 对作者:评审者会花费大量时间阅读代码,进行细致的评审,因此作者应当尊重评审意见,并通过将来帮助评审他人的更改来回馈社区。

最重要的是,保持建设性的沟通,在互动中尽可能假设对方的善意。如果发现某些流程无法顺利推进,建议直接与其他贡献者沟通,共同优化流程或沟通方式。

代码质量考量因素

高质量的代码对项目长期成功至关重要。以下是在评审中应考虑的几个关键因素:

  • F0:整体架构: 包括公共模块、关键数据结构和公共接口的定义。良好的架构设计是项目长期成功的基石。
  • F1:架构一致性: 实现功能的方法有很多,我们必须确保新功能与现有架构一致,并能良好地与现有代码协作。每个新功能都会增加项目复杂度,保持一致性可以最小化这种复杂度的上升。
  • F2:健壮性与测试覆盖率: 保证代码在各种环境下正确运行,并对新功能提供充分测试覆盖。对用户相关的错误需有清晰的报错信息。
  • F3:用户 API 文档: 公共 API 和关键模块接口必须有文档,包括 include/tvm 和面向用户的 Python 接口。我们鼓励为常用内部 API 提供适当文档,详见 F4。
  • F4:代码可读性: 包括函数命名一致且具启发性,整体流程清晰,复杂逻辑与内部函数有说明性注释。可读性强的代码更易于维护。

架构设计和一致性是最重要的因素,因为它们最可能引入长期技术债务。因此,提交者在合并代码之前应特别慎重考虑这些方面。

测试覆盖率和 API 文档是对代码贡献的基本要求。

相较于其他方面,代码可读性是一个相对主观的问题。每个人对「怎样写出好的代码」都有不同的看法。评审者应提供具有建设性和可操作性的意见。但代码审阅不应变成强迫别人照你那样写代码的过程。相反,你也应意识到你觉得没问题的内容,可能并不适合更广泛的社区。应根据代码的内容、贡献的范围以及贡献者的背景,合理判断什么是合适的。

我们在编写代码时遵循通用的代码指南与技巧。风格指南的目的是确保代码在原作者离开后依然具有可读性与可维护性。风格不仅仅是格式,还包括如何撰写文档、变量命名以及其他格式器无法自动强制执行的约定。

达成共识

在代码审阅过程中,出现分歧是可能的。我们鼓励相关人员之间建立共识。我们是一起协作、相互建立信任的开源社区。开源的本质意味着在一些次要问题上我们可能需要作出妥协,以取得稳定进展并欢迎更广泛的参与。妥协有时也意味着事情不会完全如我们所愿——即便对于社区的领导者也是如此。

  • 保持文明,并通过建设性的技术讨论来建立共识。
  • 对某个领域有了解的提交者可以作为「引导者」参与讨论,综合所有意见后提出解决方案推动进展。
  • 由于这种模式依赖于「引导者」的信任,他们在批准合并前应仔细阅读 PR。此外,合并者还应承担后续跟进责任,以应对可能因合并而产生的问题。

一致性

最后要提醒的是,我们都是人,无法永远做到完全一致。如果贡献者觉得你未能一致地应用这些指导方针,请认真倾听他们的意见。随着社区的发展,我们需要不断迭代流程和规范。我们的目标是尽力做到一致与客观,但人终究不完美,我们需要不断学习与调整。

其他建议

深思 API 与数据结构设计

一个最小、稳定的 API 对项目生命至关重要。良好的 API 能带来极大的改变。在设计时应全面思考各个方面,包括命名、参数定义和行为。

在代码审阅时应尽可能关注 API 设计。请记住,改进代码实现相对容易,而一旦某个 API 被接受,后续要更改它就非常困难。对于跨模块共享的数据结构(如 AST)也应给予同等关注。如果拿不准,建议在提交前与更多开发者讨论。

设计 API 的一些有用原则:

  • 如果功能与已有知名库重叠,尽量保持一致性。例如,张量操作的 API 应尽可能与 numpy 保持一致。
  • 与项目内已有 API 保持一致。例如,所有优化 pass 的参数顺序应统一,避免用户使用时产生「意外」。
  • 考虑 API 未来是否会发生变化。例如,随着 build 过程添加更多优化(如 loop_unrolling、device placement 策略),我们可以将优化参数打包成一个构建配置对象,使得 build API 长期保持稳定。
  • 撰写文档。API 文档是强制要求的,有时写文档的过程能帮助我们进一步思考设计是否合理、是否需要补充说明。
  • 最小化。考虑用户使用该 API 时需要写多少行代码。能减少抽象层就减少。

最小化依赖

引入依赖要格外谨慎。虽然复用已有代码、避免重复造轮子很重要,但依赖也会增加用户部署时的负担。一个良好的设计原则是:某个功能或特性应仅在用户真正使用它时才引入依赖。

简洁实现

应优先考虑:使用向量化数组代码代替循环;使用已存在的 API 解决问题。

在代码审阅中记录经验教训

如果你发现某些问题是反复出现的、具有共性的,可以将其总结并添加到代码指南与技巧。在要求改动时引用该文档,有助于将经验传达给整个社区。

学习其他人的代码审阅

同一个 PR 可能由多个审阅者共同评审,其他审阅者可能会发现你未注意到的问题,尽量从其他审阅中学习,并记录总结这些经验。

明确批准或请求更改

贡献者和代码拥有者可以邀请多个审阅者参与。记得在自己的评论被采纳后明确表示「批准」更改。操作方法:点击 PR 的「Changes」标签页,选择「Approve」表示通过,或在代码上评论并点击「Request Changes」表示请求修改。在部分审阅者未及时回复(如一周)但已有充分评审的情况下,代码拥有者可酌情决定是否合并。

审阅者职责

被请求审阅的审阅者应尽量及时给予反馈。代码审阅是项目健康的重要部分,应该被视为贡献者的常规职责。自动化工具可帮助追踪无活动的 PR,并通过机器人提醒相关人员。