LLVM 代码审查策略和实践

LLVM 的代码审查策略和实践有助于维护整个项目的代码质量。具体来说,我们的代码审查流程旨在:

  • 提高可读性和可维护性。

  • 提高健壮性并防止引入缺陷。

  • 充分利用其他贡献者在每个提议的更改中的经验。

  • 通过社区领导者的指导,帮助培养和发展新的贡献者。

所有贡献者都必须了解我们的代码审查实践并参与代码审查流程。

一般策略

哪些代码需要审查?

所有开发人员都需要在将重大更改提交到代码仓库之前进行审查。

代码必须在提交之前进行审查吗?

代码可以在提交之前或之后进行审查。我们期望在提交之前审查重大的补丁。较小的补丁(或开发人员拥有该组件的补丁)且满足可能获得社区共识的要求(适用于所有补丁批准)可以在显式审查之前提交。在存在任何不确定性的情况下,应在提交之前审查补丁。

请注意,负责补丁的开发人员也负责进行所有必要的审查相关更改,包括在任何提交后审查期间提出的更改。

代码可以在提交后进行审查吗?

鼓励提交后审查,并且可以使用下面详细介绍的任何工具来完成。我们强烈期望作者能够及时回复提交后反馈并解决问题。未能做到这一点会导致补丁被回退

如果社区成员对最近的提交表示担忧,并且此担忧在提交前审查期间足以引发讨论(包括围绕更多设计讨论的需要),他们可以要求回退到原始作者,原始作者有责任及时回退补丁。开发人员经常意见不一致,站在要求更多审查的开发人员一边可以防止代码树中出现任何持续的争议。这并不表示补丁作者有任何错误,这是我们提交后审查实践固有的。回退补丁可确保设计讨论可以在不阻塞其他开发的情况下进行;一旦问题得到解决,补丁很可能以基本上相同的方式重新应用。

在重新提交之前,补丁通常应进行进一步审查。识别问题的社区成员应积极参与审查。如果问题是由构建机器人识别,则拥有与构建机器人上类似硬件的社区成员应参与审查。

请注意:提交后反馈的标准不高于提交前反馈。不要不必要地延迟提供反馈。但是,如果您在代码提交后发现了一些您会在提交前发表评论的内容(如果您早点注意到的话),请随时提供该反馈。

话虽如此,如果自原始更改提交以来已经过了一段相当长的时间,那么创建新的补丁来解决问题可能比在原始提交上发表评论更好。例如,原始补丁作者可能不再是项目的活跃贡献者。

使用什么工具进行代码审查?

提交前代码审查在 GitHub 上使用拉取请求进行。请参阅GitHub 文档。

何时需要 RFC?

有些更改过于重大,仅靠代码审查是不够的。需要更改 LLVM 语言参考(例如,添加新的目标无关内联函数)、在 Clang 中添加语言扩展等更改,首先需要在项目的*-dev邮件列表中发送 RFC(征求意见)电子邮件。对于承诺对用户和/或下游代码库产生重大影响的更改,审查者可以请求在进行代码审查之前达成共识的 RFC。话虽如此,发布初始补丁可以帮助进行 RFC 讨论。

代码审查工作流程

代码审查可能是一个迭代过程,直到补丁准备好提交为止。具体来说,一旦发送出补丁以供审查,它需要在提交之前获得明确的批准。不要假设默许批准,也不要以截止日期为条件征求对补丁的异议。

注意

如果您出于审查以外的目的使用拉取请求(例如:提交前 CI 结果、方便的基于 Web 的回退等),请在 PR 中添加skip-precommit-approval标签。

确认所有审查者反馈

补丁作者应确认审查者提供的所有评论。通常预期会将建议的更改合并到补丁的未来版本中,除非作者和/或其他审查者可以阐明一个充分的理由来拒绝(然后审查者必须同意)。如果新的补丁没有解决所有未决的反馈,则作者在提供更新的补丁时应明确说明这一点。当使用基于 Web 的代码审查工具时,此类注释可以在“差异”描述中提供(这与用于提交消息的“差异修订”的整体描述不同)。

如果您在代码审查中建议更改,但又不希望该建议被如此强烈地解读,请明确说明。

注意

在回复审查者评论后,请点击重新请求审查,以提请审查者注意拉取请求。

旨在有效利用每个人的时间

旨在限制审查过程中的迭代次数。例如,在建议更改时,如果您希望作者在代码的其他位置进行类似的更改,请解释请求的更改集,以便作者可以一次性进行所有更改。如果补丁在批准之前需要多个步骤(例如,拆分、重构、发布来自特定性能测试的数据),请尽可能提前解释这些步骤。这允许补丁作者和审查者以最有效的方式利用他们的时间。

LGTM - 如何接受补丁

当审查者接受补丁时,该补丁即可批准提交,这几乎总是与包含文本“LGTM”(代表“看起来不错”)的消息相关联。只需要一位审查者的批准。

在提供无条件的 LGTM(批准提交)时,审查者有责任审查所有讨论和来自所有审查者的反馈,确保已解决所有反馈,并且所有其他审查者几乎肯定会对补丁获得批准感到满意。如果不确定,审查者应提供有条件的批准(例如,“LGTM,但请等待 @someone、@someone_else”)。如果您相当确定某个特定社区成员希望进行审查,即使该人尚未进行审查,您也可以这样做。

请注意,如果审查者已请求某个特定社区成员进行审查,并且该社区成员在一周后仍未回复,请随时 ping 补丁(字面意思是提交带有“Ping”字样的补丁评论),或者,请求原始审查者提供进一步的建议。

如果其他人可能希望审查最近发布的补丁,尤其是在可能存在异议的情况下,但还没有人这样做,那么提供有条件的批准(例如,“LGTM,但请等待几天以防其他人希望审查”)也是礼貌的。如果很快收到批准,补丁作者也可以选择在提交之前等待(对于非平凡的补丁,这当然被认为是礼貌的)。特别是考虑到我们社区的全球性,此等待时间应至少为 24 小时。请同时注意周末和主要节假日。

我们的目标是确保围绕设计决策和重大实现选择达成社区共识,而审查者在为补丁提供整体批准时的责任之一是合理地确保存在这种共识。如果您对社区不够熟悉,无法知道,那么您不应该为提交提供最终批准。提供最终批准的审查者应具有对 LLVM 项目的提交权限。

每个补丁都应至少由受更改影响的项目领域中的一位技术专家进行审查。

拆分请求和条件接受

审查者可能会请求将补丁的某些方面分解为单独的补丁以进行独立审查。审查者还可以接受补丁,但前提是作者提供后续补丁来解决某些特定问题或疑虑(尽管任何已提交的补丁都不应使项目处于损坏状态)。此外,审查者可以接受补丁,但前提是作者在提交之前应用一些次要更新,并且在适用情况下,审查者这样做是礼貌的。

不要无意中阻止审查

如果您审查了补丁,但又不打算让审查过程阻塞您的批准,请明确说明。出于礼貌,我们通常会等待提交补丁,直到所有审查者都满意为止,如果您不打算及时查看补丁,请在审查中说明这一事实。

谁可以/应该审查代码?

非专家也应该审查代码

您无需成为代码库某些领域的专家即可审查补丁;询问某些代码段的作用是可以的。如果对您来说不清楚发生了什么,那么您可能不是唯一一个不清楚的人。请记住,从长远来看,让只有少数人才能很好理解的组件不利于社区。额外的注释和/或测试用例通常可以提供帮助(并且要求在测试用例中添加注释也是可以的)。

此外,鼓励作者将问题视为重新审视相关代码可读性的理由。结构上的更改或进一步的注释可能是合适的。

如果您是 LLVM 社区的新成员,您可能会发现此演示文稿也很有用:.. _如何为 LLVM 做贡献,2019 年 LLVM 开发者会议演示文稿:https://youtu.be/C5Y977rLqpw

对于新的贡献者来说,提高他们对代码库的了解的一个好方法是审查代码。完全可以审查代码并明确地将批准决定权委托给其他人。

专家应审查代码

如果您是编译器某个受提议补丁影响的领域的专家,那么强烈建议您审查代码。如果您是相关的代码所有者,并且没有其他专家审查补丁,则您必须帮助安排专家审查补丁或亲自审查。

代码审查、速度和互惠

有时代码审查所需时间可能比您希望的要长,特别是对于较大的功能而言。加快补丁审查时间的常见方法是

  • 审查其他人的补丁。如果您提供帮助,每个人都将更愿意为您做同样的事情;善意是我们的货币。

  • Ping 补丁。如果紧急,请说明您希望此补丁尽快合并的原因,并每隔几天 Ping 一次。如果不是紧急情况,常见的礼貌 Ping 频率为一周一次。请记住,您正在请求其他专业开发人员宝贵的时间。

  • 在 IRC 上寻求帮助。IRC 上的开发人员将能够直接帮助您,或者告诉您谁可能是合适的审查者。

  • 将您的补丁拆分为多个相互构建的小补丁。您的补丁越小,有人快速查看它的可能性就越高。在执行此操作时,在每个补丁系列的标题中添加“[N/M]”(对于 1 <= N <= M)很有帮助,以便清楚地表明存在顺序以及该顺序是什么。

开发人员应以审查者和作者的身份参与代码审查。如果有人友好地审查了您的代码,您应该为其他人提供回报。请注意,任何人都欢迎审查并对补丁提供反馈,但补丁的批准应与上述政策保持一致。