审查贡献#

原则#

Arrow 是一个基础项目，需要在未来数年甚至数十年内不断发展，并可能服务于数百万用户。我们相信，在审查时一丝不苟比宽松并追求快速合并对项目更有益。

像这样的代码审查可以带来更高质量的代码，更多参与并理解代码变更的人员，以及总体上更健康的项目，拥有更大的发展空间以适应新兴需求。

准则#

元#

**运用你的判断力**。这些准则并非硬性规定。提交者应在其工作领域具备足够的专业知识，能够根据他们所关注的问题调整其方法。

这些准则没有特定的顺序，也不打算用作清单。

最后，这些准则并非详尽无遗。

范围和完整性#

我们的一般策略是不引入回归或合并需要后续操作才能正常运行的 PR（尽管可以例外）。合并后进行必要的更改对于贡献者和审查者来说成本更高，对于其他可能因合并的 PR 引入的问题而感到困惑的开发人员来说也是如此。
PR 的范围内有哪些更改，以及哪些更改可能/可以/应该被推到范围外并创建后续问题，应由作者和审查者协商确定。
当贡献一个大型功能并且希望将其分段集成时，在决定如何划分更改时，应优先考虑功能的内聚性（例如，如果贡献了一个文件系统实现，则第一个 PR 可以贡献目录元数据操作，第二个 PR 文件读取工具，第三个 PR 文件写入工具）。

公共 API 设计#

公共 API 应引导用户使用最理想的结构。换句话说，如果有一种“最佳”方法来做某事，那么理想情况下，它也应该是最容易发现和键入最简洁的方法。例如，安全的 API 应该比不安全的 API 更突出，不安全的 API 可能会在无效输入时崩溃或静默地产生错误结果。
公共 API 理想情况下应倾向于生成可读的代码。一个例子是当预期随着时间的推移添加多个选项时：最好尝试逻辑地组织选项，而不是将它们全部并列在函数的签名中（例如，参见 C++ 和 Python 中的 CSV 读取 API）。
命名很重要。试着问问自己，调用新 API 的代码是否可以在不阅读 API 文档的情况下理解。应避免使用模糊的命名；不准确的命名更糟糕，因为它会误导读者并导致用户代码出现错误。
注意术语。每个项目都有（明确或默认）关于如何命名重要概念的约定；偏离这些约定会增加贡献者和项目用户的认知负担。相反，将一个众所周知的术语用于与其通常不同的目的也会增加认知负担，并使开发人员的生活更加困难。
如果您不确定 API 是否适合手头的任务，建议将其标记为实验性的，以便用户知道它可能会随着时间的推移而改变，而贡献者则不那么担心引入破坏代码的改进。但是，实验性 API 不应用作回避基本 API 设计原则的借口。

稳健性#

Arrow 是一组开源库，将在非常广泛的环境中使用（包括在 Jupyter 解释器提示符下处理刻意构造的数据）。如果您正在编写公共 API，请确保它不会在异常（但有效）输入时崩溃或产生未定义的行为。
当实现一个非平凡的算法时，防御性编码可以用来捕捉潜在的问题（例如仅限调试的断言，如果语言允许的话）。
摄取潜在不可信数据的 API，例如磁盘上的文件格式，应尽量避免在输入无效或损坏的数据时崩溃或产生静默错误。这可能需要大量工作，超出了常规代码审查的范围（例如设置模糊测试），但在代码审查阶段仍然可以建议进行基本检查。
调用外部 API 时，尤其是系统函数或处理输入/输出的 API，请务必检查错误并传播它们（如果语言不自动传播错误，例如 C++）。

性能#

考虑性能，但不要过分沉迷于此。如果输入大小可能“很大”，那么算法复杂性很重要（“大”的含义取决于上下文：使用您自己的专业知识来决定！）。对性能敏感的功能进行微优化，使其性能提高 20% 或更多，可能也很有用；较小的微优化可能不值得花费时间，尤其是如果它们导致代码更复杂的话。
如果性能很重要，请对其进行测量。不要满足于猜测和直觉（这可能是基于对编译器或硬件的错误假设）。

另见

Arrow 基准测试
尽量避免试图通过以某种方式编写代码来欺骗编译器/解释器/运行时，除非它真的很重要。这些技巧通常很脆弱，并且依赖于可能过时的平台细节，它们还会使代码更难维护（一个常见的阻碍贡献者的问题是“我应该如何处理我的更改想要删除的这个奇怪的技巧”？）。
避免粗糙的边缘或退化行为（例如当大小估计不准确地大时内存爆炸）可能比试图少量改进常见情况更重要。

文档#

这些准则理想情况下应适用于散文文档和代码内文档字符串。

寻找模棱两可/信息量不足的措辞。例如，*“如果...则是一个错误”* 的信息量不如 *“如果...则会引发错误”* 或 *“如果...则行为未定义”* （第一种说法没有告诉读者在出现此类错误时实际会 *发生什么*）。
在审查文档更改（或一般情况下的散文片段）时，请注意拼写、语法、表达和简洁性。清晰的沟通对于与来自不同背景的人进行有效协作至关重要，并有助于改进文档。
一些贡献者的母语不是英语（也许你也不是）。建议帮助他们和/或在需要时寻求外部帮助。
交叉链接增加了文档的整体价值。Sphinx 特别具有强大的交叉链接功能（包括主题引用、词汇表术语、API 引用），因此请务必使用它们！

测试#

添加 API 时，所有标称情况都应该有测试用例。函数是否允许空值？那么应该测试空值（当然还有非空值）。函数是否允许不同的输入类型？等等。
如果功能的某些方面很微妙（无论是定义上的还是实现细节上的），都应该对其进行测试。
应该测试极端情况，尤其是在低级实现语言（如 C++）中。例如：空数组、零块数组、仅包含空值的数组等。
压力测试可能很有用，例如，如果添加了重要的并行化，则可以发现同步错误，或者可以使用缓慢而直接的参考实现来验证计算参数。
然而，一个缓解因素是运行测试套件的总成本。持续集成 (CI) 运行时间可能非常长，我们应该警惕过度增加它们。有时值得微调测试参数，以平衡测试的有用性和运行它们的成本（尤其是在涉及压力测试的情况下，因为它们往往意味着对大型数据集执行）。

标签#

在审查 PR 时，我们应该尝试确定相应的问题是否需要标记以下一个或两个问题标签

**严重修复**：此更改修复了以下问题：(a) 安全漏洞；(b) 导致产生不正确或无效数据的错误；或 (c) 导致崩溃的错误（同时维护 API 约定）。这旨在标记可能在用户不知情的情况下影响用户的问题的修复。因此，修复导致错误的 bug 不算在内，因为这些 bug 通常很明显。导致崩溃的错误被认为是严重的，因为它们是拒绝服务攻击的可能载体。
**重大更改**：此更改破坏了公共 API 中的向后兼容性。对于 C++ 中的更改，这不包括仅仅破坏 ABI 兼容性的更改，除了少数我们确实保证 ABI 兼容性的情况（例如 C 数据接口）。实验性 API *不* 免除于此；它们只是更有可能与这个标签相关联。

重大更改和严重修复是分开的：重大更改会更改 API 约定，而严重修复会使实现与现有的 API 约定保持一致。例如，修复导致 Parquet 读取器跳过包含数字 42 的行的错误是一个严重修复，但不是一个重大更改，因为行跳过不是一个理性用户会依赖的行为。

这些标签用于在发布版本中突出显示用户在考虑升级库版本时应该注意的更改。重大更改有助于确定用户可能希望等到有时间调整其代码后再升级的原因，而严重修复则突出了*不*升级的风险。

此外，我们使用以下标签来指示优先级

**优先级：阻碍者**：指示在下一个版本发布之前*必须*合并更改。这包括对测试或打包失败的修复，这些失败会阻止发布成功完成最终打包或验证。
**优先级：严重**：指示高优先级问题。这是标记为“严重修复”的问题的超集，因为它还包含对导致错误和崩溃的问题的某些修复。

协作者#

协作者角色允许用户被赋予分类角色以便能够帮助分类问题。该角色允许用户标记和分配问题。

用户可以请求成为协作者，或者在项目中合作一段时间后由其他社区成员提名。合作可以包括但不限于：创建 PR、回答问题、创建问题、审查 PR 等。

要提名某人为协作者，您必须创建一个 PR，将该用户添加到 .asf.yaml 上的协作者列表中。提交者可以审查该用户的过去合作并批准。

长时间不活跃的协作者可以从协作者列表中删除。