Path: blob/master/site/zh-cn/xla/code_reviews_guide.md
25115 views
代码审核指南
本文档的目的是说明 XLA 团队在代码审核方面的立场背后的原因 – 这一立场源于多年从事开源项目(特别是 XLA)的集体经验。
不同的开源项目对于审核者可以对代码作者提出多少要求有着不同的文化期望。在某些项目中,审核者会接受“大部分正确”的拉取请求 (PR),自行修改并提交。XLA 采用相反的方式:我们希望作者在 PR 上进行迭代,直到它们足够好,无需额外更改即可提交。
这种方式的主要原因是我们希望 PR 作者学习成为成熟的 XLA 贡献者。如果审核者自行修正 PR 中的问题,那么作者学习起来会相当困难。XLA 方式对审核者和被审核者都极具挑战性,但我们相信它最终会帮助我们发展社区。
学习成为“成熟的 XLA 贡献者”不仅仅是编写没有错误的代码,还有更多关于“如何修改 XLA”的知识。这包括:
编码风格
需要注意哪些边界情况
对编写测试的期望
对注释和 PR 描述的期望
对构建基础架构以支持您的更改的期望
随着您对项目的了解和审核者的信任,您可以期望获得更少的注释,因为您会自然而然地编写更符合审核者期望的代码。
像许多开源项目一样,XLA 拥有一些经验丰富的人和许多相对较新的人。我们这些经验丰富的人对时间有很多要求。为了使 PR 及时向前推进,您可以通过下面的做法帮助减少审核者所需的时间和所需的迭代次数:
在发送 PR 之前仔细审核和/或让同事审核您的 PR:在发送 PR 以供审核之前,尝试删除尽可能多的无足轻重的错误(代码风格、拼写和语法错误等)。确保所有测试都通过。
仔细阅读审核者的注释:在推送新版本之前,尝试了解审核者的要求并尝试解决所有注释。
避免离题的讨论(帕金森琐碎定理):技术讨论和分歧非常有价值,人无完人。但是,请避免没有影响或仅仅是风格的讨论。如果您不同意审核者的注释,请尝试尽可能准确和全面地详细说明您的理由,以避免冗长的反复讨论。
避免询问下面列出的“常见审核问题”:我们在下面列出了常见问题的一些回答和我们的理论依据。
一般来说,我们邀请您尽量减少审核您的 PR 所花费的时间。然后我们希望快速审核您的更改!
感谢您为 XLA 做出贡献,祝您编程愉快!
常见审核问题
“这个基础架构的更改和我的 PR 无关,我为什么要这么做?”
XLA 团队没有专门的基础架构团队,因此构建辅助函数库和避免技术负债取决于我们所有人。我们认为这是对 XLA 进行更改的常规部分,并且每个人都应该参与。我们通常在编写代码时根据需要构建基础架构。
XLA 审核者可能会要求您构建一些基础架构(或以其他方式对 PR 进行重大更改)以及您编写的 PR。这项请求可能看起来不必要或与您尝试进行的更改无关。这可能是因为您对需要构建多少基础架构的期望与您的审核者对此的期望不匹配。
期望不匹配没有问题!当您是一个项目的新手时,这是可以预期的(有时甚至会发生在我们这些老手身上)。您过去从事的项目可能有不同的期望。这也没关系,意料之中!这并不意味着这些项目中的任何一个存在错误的方式;它们只是有些不同。我们邀请您将基础架构请求与所有其他审核注释一起作为了解我们对该项目的期望的机会。
“我可以在未来的 PR 中解决您的注释吗?”
关于 PR 中的基础架构请求(或其他大型请求)的一个常见问题是,是否必须在原始 PR 中进行更改,或者是否可以在未来 PR 中进行后续跟进。
一般来说,XLA 不允许 PR 作者将审核注释作为后续 PR。当审核者决定需要在此 PR 中解决某些问题时,我们通常希望作者在原始 PR 中解决它,即使请求的更改非常大。这项标准同时适用于 Google 内外部。
XLA 采用这种方式的原因如下。
信任:赢得审核者的信任是一个关键组成部分。在开源项目中,贡献者可以随意出现或消失。在我们批准 PR 后,审核者无法确保任何承诺的后续行动真正完成。
对其他开发者的影响:如果您发送了涉及 XLA 特定部分的 PR,那么其他人很有可能正在查看相同的部分。如果我们在您的 PR 中接受技术负债,那么在提交后续行动之前,查看此文件的每个人都将受到该负债的影响。
审核人带宽:将更改推迟到后续行动会给我们已经超负荷的审核者带来多重成本。审核者可能会在等待跟进的过程中忘记此 PR 是关于什么内容,从而使下一次审核变得更加困难。此外,审核者必须跟踪他们正在等待的后续行动,以确保这些行动确实发生。如果可以进行更改以使其真正与原始 PR 相关,以便其他审核者可以对其进行审核,那么带宽问题便不会那么严重。根据我们的经验,这种情况十分少见。