审阅贡献#

原则#

Arrow是一个基础项目，需要在未来许多年甚至几十年里不断发展，同时服务于可能数百万的用户。我们认为，在审查时一丝不苟会给项目带来更大的回报，而不是宽松处理并追求快速合并。

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

指南#

元数据#

运用自己的判断。这些指南不是硬性规定。提交者应具备其工作领域的足够专业知识，能够根据任何疑虑调整其方法。

这些指南没有按照特定顺序排列，也不打算用作检查清单。

最后，这些指南并不详尽。

范围和完整性#

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

公共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时，我们应该尝试确定相应的issue是否需要使用以下一个或两个issue标签进行标记

关键修复：更改修复以下内容之一：(a) 安全漏洞；(b) 导致生成不正确或无效数据的错误；或 (c) 导致崩溃的错误（同时遵守API契约）。这旨在标记可能在用户不知情的情况下影响用户的错误修复。因此，修复导致错误的错误不计入，因为这些错误通常很明显。导致崩溃的错误被认为是关键的，因为它们可能是拒绝服务攻击的潜在媒介。
破坏性更改：更改破坏了公共API中的向后兼容性。对于C++中的更改，这并不包括仅破坏ABI兼容性的更改，除了我们确实保证ABI兼容性的少数几个地方（例如C数据接口）。实验性API不豁免此项；它们只是更有可能与此标签相关联。

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

这些标签用于在发布版本中突出显示用户在考虑升级库版本时应该注意的变化。重大更改有助于识别用户可能希望等待升级的原因，直到他们有时间调整其代码，而关键修复则突出了**不**升级的风险。

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

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

协作者#

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

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

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

一段时间内未活跃的协作者可以从协作者列表中移除。