审查拉取请求

在审查 PR 时，我们的首要目标是与社区一起改进 DataFusion 及其社区。对 PR 的反馈应当具有建设性，既帮助改进代码，也帮助贡献者加深理解。

审查带宽是我们目前最稀缺的资源，因此我们非常欢迎并鼓励更广泛的社区成员参与审查。审查 PR 是了解代码库的绝佳途径，而且你不需要成为提交者（committer）也能给出有价值的审查意见。事实上，成为提交者最好的方式之一，就是用心地审查他人的 PR。

请确保你留下的每条评论都包含理由和建议的替代方案——如果只被告知“别这么做”，却不给出任何明确的理由或替代做法，会让人非常沮丧。

本指南中的这些标准，同样可以作为你在准备自己的 PR 送审时的实用清单。

PR 审查流程

一些有用的链接：

GitHub 上等待审查的 PR
GitHub 上已批准、等待合并的 PR

PR 的整体生命周期（触发 CI、批准、“重大” PR 的 24 小时规则以及合并）在贡献者指南的 PR 概览部分有详细说明。

实用技巧：

将变更检出到本地，在你的 IDE 或智能体中进行探索，例如使用 GitHub CLI 执行 gh pr checkout <PR 编号>。
通常无需在本地重新运行 CI 已经跑过的测试。
尽可能针对 diff 的具体代码行留下评论，这样讨论才有上下文。
如果你审查了某个 PR 但不确定是否应该批准，留下评论同样很有价值：部分审查（例如“我看了测试，没问题”）能帮助下一位审查者更高效地分配时间。
任何不需要阻塞当前 PR 的问题，都可以记录为后续待办（最好创建一个 issue），从而让 PR 保持聚焦、尽快合并。

审查 PR 描述

当用户和贡献者想了解某次变更背后的意图，或者代码本身不够清晰时，PR 描述往往是他们首先会查阅的内容。PR 描述最终也会成为扩展的提交信息（commit message）。

检查描述是否：

简明扼要地从用户视角描述所要解决的问题。
遵循 PR 模板，并回答模板中的问题。
准确描述 PR 的内容，包括相关的上下文或背景信息。优秀的描述具有很高的信噪比，会总结重要的实现变更，而不会重复代码中已有的技术细节。
明确指出任何面向用户或 API 的变更（参见下文的审查代码）。

审查测试覆盖

检查该功能或修复是否有充分的测试覆盖（更多详情参见测试指南）：PR 应当包含新功能的测试，而缺陷修复则应当包含一个能复现所报告问题的测试。

评估测试的指导原则：

尽可能优先使用 sqllogictest（.slt）测试或 DataFrame API 测试，因为它们验证的是用户可见的行为，与单元测试相比，对内部实现细节的耦合更少。
确认测试覆盖了边界场景和常见的失败情况，而不仅仅是常见的成功路径。不过，并没有必要测试每一条可能的出错路径，尤其是当这些路径难以触发或在实践中不太可能发生时。
通过 PR 上的 codecov 检查，或在本地运行 cargo llvm-cov 生成 HTML 报告，来验证变更代码的测试覆盖情况。对未覆盖的行要自行判断——目标是对变更建立信心，而不是死板地追求某个覆盖率数字。
避免出现大量重复样板代码的测试：当许多测试共享几乎相同的设置时，很难看出它们之间的差异（也就难以看出究竟在测试什么）。要让各个用例之间的差异一目了然。
检查测试是否针对具体的预期值或执行计划进行断言（例如通过 insta 快照或 .slt 的预期输出），而不是仅仅检查"没有发生错误"。
验证测试确实覆盖了该缺陷（"消融测试"）：对于缺陷修复，先在本地回退该修复，然后确认没有修复时新测试会失败（即测试确实能够复现该缺陷或覆盖该新功能）。

审查代码

检查以下内容：

代码清晰，并与现有代码库的风格保持一致。
新增的函数和测试应放置在相似函数和测试的附近。例如，辅助函数应定义在使用位置附近，新测试应与被测试代码位于同一模块中。SLT 测试应放入功能相关的现有 .slt 文件中，除非新测试足够多，值得为其单独创建一个文件。
新 API 与现有的公共 API 及模式保持一致；当已存在类似机制时，PR 应扩展该机制，而不是另起炉灶引入并行机制。
对公共 API 的任何更改都遵循 API 健康政策。
变更范围应适当：无关的重构、格式化改动或顺带的修改会使审查变长，最好拆分为独立的 PR。
新增的错误信息应具有可操作性，指明出错的对象，并使用正确的错误变体（例如，用户可触发的错误使用 plan_err!，而不变量被破坏时使用 internal_err!）。

审查性能

性能是 DataFusion 的一项关键特性。有关项目政策，请参阅性能改进：改进应当“足够好”，以证明所增加的代码复杂性是值得的，并且性能类 PR 应附带基准测试结果。

在审查时：

查找相关的现有基准测试并在 main 分支上运行它们：系统级 SQL 基准测试通过 bench.sh 运行（参见基准测试 README），微基准测试（例如位于 datafusion/functions/benches 中的测试）通过 cargo bench 运行。
注意，在同时运行其他任务的机器上进行基准测试会使结果难以复现。请优先使用安静、专用的机器并多次运行。
如果 PR 声称带来了性能提升，请检查所报告的结果是否可复现，以及基准测试是否覆盖了被修改的代码路径。

审查者的最佳实践

以下是审查 PR 时建议遵循的一些最佳实践。

审查语气：感谢贡献者并具体赞扬出色的工作

在审查开始时，请具名感谢作者；当一个 PR 做得很好时，要具体说明它好在哪里——正面的反馈能鼓励人们持续贡献，并帮助他们理解项目所看重的价值。

明确说明批准条件

如果你还没有准备好批准，请具体列出在批准之前你还需要看到什么（例如“基准测试结果和升级指南条目”），这样作者就有明确的合并路径。

将非阻塞性工作推迟到后续议题

明确将非关键的建议推迟到后续的 PR，并为它们创建（或要求作者创建）议题，这样优秀的 PR 才能快速合并，同时避免范围蔓延。

类似地，当一个 PR 混合了重构与行为变更，或者用一个宽泛的机制来修复一个狭窄的问题时，应要求将其拆分或缩小范围，而不是照原样进行审查。

批准时说明你验证了什么

与其只留下一句“LGTM”，不如说明你实际检查了什么（“手动追踪了状态转换”、“确认哈希器的更改不会影响排序”），这样就能清楚地知道哪些内容已被验证，哪些没有。

在核心改动上邀请其他提交者

对于核心的、被广泛共享的代码的改动，即使你已经批准，也要让这个 PR 保持开放状态，供其他提交者查看，并抄送那些熟悉该领域的人员。

审阅 Pull Request