代码 Reviews
开源代码由具有不同背景、兴趣和目标的社区维护。因此,提供清晰、文档化和可维护的代码和流程非常重要。代码 review 是一个引导过程,用于共同发现潜在问题,提高代码质量,并就代码库及其假设对贡献者和 reviewer 进行教育。这也是确保多人共同维护相关代码的一种机制。鼓励贡献者在请求 review 之前将代码完善到可 review 的状态。这对于 committer 候选人来说尤其重要,因为 committer 不仅要参与写代码,还要参与 review。
本文档是开源代码 review 的在线指南。还请花一些时间阅读 TVM 社区指南 的一般开发过程。
建立信任
一个基于信任的社区,需要时间和精力来建立和维护。我们希望社区成员能够达成共识,并且以一种建设性的方式来协作。尽管我们都有着不同的背景、兴趣和目标,但我们必须共同努力寻找 适合更大社区的解决方案。基于信任的协作不仅是 Apache 的关键租户,它对于发展社区,以及将社区成员提升为官方角色来说也很重要。
社区参与
欢迎大家对 PR 发表评论。我们鼓励 committer 在 merge 包含重大架构更改的 PR 之前等一段时间(例如三天),其目的是让人们有时间畅所欲言、充分讨论他们对 review 的兴趣并参与其中。
我们来自的背景不同,这对社区参与来说很重要。例如,一些社区成员工作在不同的时区,只在工作时间从事开源工作,其他时间可能在旅行或进行其他活动。在大型项目中工作有着集体认知是很重要的,这样的话没有人会成为瓶颈。虽然留出时间参与代码 review 很重要,但我们也不能阻止所有 reviewer 的所有更改。记住,帮助人们获得 PR 是鼓励更广泛参与的好方法,尤其是对于那些自愿贡献时间的人。
其中一部分是与其他 maintainer 的信任和沟通,如果将来要应用更改,PR 的作者要兑现他们的承诺。committer 有义务听取任何来自 PMC 成员或新贡献者的反馈,并考虑要采取什么行动。
仔细阅读代码
有时我们会快速通读代码,只关注代码的某一部分,社区应该欢迎这些类型的评论。不过,它们只是代码 review 的一部分,也应成为更全面的反馈的一部分。要仔细地进行代码 review 会耗费大量时间,这个过程花费的时间可能比实际编写代码的时间还要长。
例如,就算只是收到关 于 PR 次要方面的高度批评性反馈,都会让人觉得投入的时间和精力在 review 期间没有得到回报,这会让他们非常沮丧。不管是作为贡献者还是 committer 时,同理心都很重要,它可以帮助你成为更有效的代码 reviewer 和贡献者。
我们希望所有 committer 在签名(sign off)之前仔细阅读并理解代码。当 committer 点击 merge 按钮时,牵涉到很多人对他的信任。同时,我们承认有时会出现问题,在这种情况下,merger 有责任确保采取正确的后续行动。
尊重他人
- 所有发表评论的人:提出建设性评论不仅有助于新贡献者及时获得 PR,还有助于提高新成员的参与度。
- 写给作者:reviewer 会花大量时间阅读代码,仔细 review 可能与从头开始编写代码一样耗费时间。恭敬地处理 review 意见,并在将来 review 其他人的代码,来回报他们的 review。
最重要的是专注于进行建设性的对话,并在作为 reviewer 与其他人互动时尽量把他人往好处想。如果流程中有些东西不起作用,可以考虑与其他贡献者面对面交流,并讨论如何改进流程或沟通。
代码质量需要考虑的因素
高质量代码对于项目的长期发展至关重要。在代码 review 期间,需要从多方面对代码质量进行评估:
- F0:整体架构。这包括公共模块、关键数据结构和公共接口的定义。从长远来看,良好的架构选择对于 项目的成功至关重要。
- F1:架构一致性。通常有多种方法可以实现新功能。我们必须确保新功能与之前的整体架构选择保持一致,并能与现有代码进行良好的交互。每个新功能都会增加项目的复杂度,而一致的设计可以最大限度地降低新功能的复杂度,从长远来看利于代码维护。
- F2:代码稳健性和测试覆盖。确保代码在所有可能的场景(平台)中正确运行,确保新功能的测试覆盖。为遇到报错的用户清除错误消息。
- F3:面向用户的 API 文档:请务必编写面向用户的公共 API 和关键模块接口的文档。这包括 API、出现在公共接口中的数据结构(即 include/tvm 和面向用户的 python API)。我们鼓励代码对应有完善的文档(其中包含对内部 API 的描述,这些内部 API 在多个地方被使用),另请参阅 F4。
- F4:代码可读性。可读性涉及多个方面:有指导意义且一致的函数名称、整体流程的清晰实现、复杂代码逻辑和内部函数的描述性注释。可读性高的代码更容易维护。
架构设计和一致性至关重要,因为它们可能会引起技术问题的长期累积。因此,committer 在合并代码之前应该仔细地考虑这些因素。
代码贡献需要包含测试覆盖和 API 文档。
与其他问题相比,代码可读性是一个相对主观的问题。不同的人对如何写出最好的代码有不同的看法。reviewer 应提出一些有建设性和可操作的意见。同时,代码 review 不应该成为,一种让其他人完全按照 reviewer 的方式来写代码的方式。相反,还要考虑到一点,就是你认为很容易理解或可以接受的东西,可能不适用于更大的社区或其他成员。请根据贡献的内容和范围,以及贡献者的来源来判断什么是合适的。
我们写代码要遵循通用的 代码指南和提示。风格指南可以使得,即便在原作者离开很久之后,其他人仍可以阅读和维护代码。风格指南不仅仅关乎代码的格式化,还记录——代码、命名变量,以及没有被自动格式化程序强制执行的约定——的正确方式。
共识构建
在代码 review 期间可能会发生分歧。我们鼓励在相关人员之间建立共识,同时我们也致力于在 OSS 中彼此建立信任。 OSS 的性质意味着,我们为了取得稳步进展,有时会在不太重要的问题上做出妥协,同时欢迎更广泛的社区参与。但是妥协意味着有些事情不完全如我们所想,对社区领袖来说也是如此。
- 在建设性的基于技术的对话中保持文明并建立共识。
- 领域所属的 committer 可以充当主持人,通过考虑所有对话来推动讨论,并提出推进的解决方案。
- 因为赋予了 committer(主持人)很大的信任,所以他们应该在签署之前仔细阅读 PR。此外,如果合并引发了问题,那么合并者也应承担后续的责任。
一致性
最后一点,我们都是人,很难始终保持一致。如果贡献者认为某些地方你没有遵循一致性方针,请根据具体情况进行相应调整。随着社区的发展,流程和指导方针将会不断地迭代。我们的目标是努力保持一致和客观。
其他建议
仔细考虑 API 和数据结构
最小且稳定的 API 对项目而言至关重要。好的 API 会产生很大影响。始终要仔细考虑所有方面,包括命名、参数定义和行为。
如果可能,在代码 review 过程中仍要多关注已经提出的 API 设计。请记住,改进代码实现是很容易的一件事情,但是,一旦 API 被接受就很难更改变了。我们应该以相同的方式处理贯穿多个模块共享的数据结构(例如 AST)。如果不确定的话,请在 commit 之前与其他开发人员充分交流探讨。
以下是一些对于设计 API 而言,非常实用的原则:
- 如果功能重叠,请与现有知名包的 API 保持一致。例如,张量算子 API 应始终与 numpy API 保持一致。
- 与同一项目中的现有 API 保持一致。例如,我们应该在整个优化过程中使用相同的参数排序,这样在使用它们时才不会有发生“意外”。
- 想想未来 API 会不会发生变化。例如,随着在构建中添加更多的优化,我们会有更多选项,例如 loop_unrolling 和设备放置策略。我们可将优化 knob 打包到构建配置对象中,通过这种方式构建的 API, 即使在未来可能会被丰富,但长久来看也是稳定的。
- 写文档。文档对于 API 来说是强制性的,有时候写文档可以帮助我们进一步思考设计,以及是否需要添加进一步的说明。
- 最小化。想想用户为了使用 API 要写多少行代码。尽可能删除抽象层。
最小化依赖
引入依赖时始终要保持谨慎。虽然代码复用和避免重新造轮子很重要,但依赖关系会增加用户在部署中的负担。一个好的设计原则是,对于某个特性或功能,应该只有在用户实际使用它时,才具有依赖关系。
简洁的实现
这里的基本原则是:多用向量化数组代码而不是循环;使用现有 API 解决问题。
记录代码 review 中的经验
对于一些常见或反复出现的问题,请将其添加到 代码指南和提示 中。请求更改时可参考指南文档,这有助于将经验分享到所有社区。
从其他代码 review 中学习
可以有多个 reviewer review 相同的更改。很多时候,其他 reviewer 可能会发现你没有发现的问题。尝试从其他代码 review 中学习并积累经验。
明确同意和请求更改
贡献者和代码所有者可 以向多个 reviewer 请求代码 review。当你的评论在代码 review 中变为已解决时,记得同意更改:单击 pull request 中的 changes 选项卡,然后选择 approve;或者评论代码,然后单击 request changes。如果某些 reviewer 没有及时回复(例如一周)并且现有 review 足够,代码所有者可以决定是否 merge 代码。
Reviewers
reviewer 应及时对请求 review 的 pull request 给出反馈。review 代码是项目健康的重要组成部分,应被视为贡献者的常规责任。自动化工具可用来改善这个问题:在一段时间内没有任何活动的 PR,会收到一条机器人评论提示相关责任方。