审查补丁

来自 PostgreSQL 维基
跳转到导航跳转到搜索


PostgreSQL 开发组织成 CommitFest 周期，需要大量的审阅者来帮助处理所有提交的内容。许多人认为他们没有资格对提交给 PostgreSQL 的补丁进行全面审查。但审查包含许多不同的任务，即使你不能完成所有任务，你也可以通过承担早期阶段来帮助项目。如果你可以应用补丁，并且可以使用新功能，那么你已经具备了开始审查的资格。

最终的提交者将在补丁进入代码库之前进行自己的审查。审阅者的任务是通过发现简单问题来减轻提交者的负担。审阅者的工作不一定是为了保证某种质量水平，而仅仅是报告他们能够找到的任何问题。如果你认为补丁已准备好接受提交者的深入审查，那么这项任务就完成了。请查看 这个补丁审查 作为一项彻底审查可能会产生的输出的一个例子。当然，其他补丁的审查可能包含不同的部分，或者看起来完全不同。

另请参见 Josh Tolley 的演示文稿 (2009) 以了解对谁受欢迎的含蓄但并不不准确的描述，以及 补丁审查的审查 (2011) 或 为了乐趣和利润审查 PostgreSQL 补丁 (2016) 以更认真地了解这里涵盖的内容。

战术要点

  • 如果当前正在进行 Commitfest，它可以在 这里 获取。
  • 每个 Commitfest 应该都有一个 Commitfest 管理员，你可以向他们寻求帮助，选择补丁或处理程序方面的事宜。
  • 你无需请求许可即可自行注册。
  • 请在你计划进行审查时尽快注册。（有些人会等到实际完成审查后才注册。请不要这样做；这对我们规划审查资源没有帮助。）
  • 另一方面，我们要求在约五天内提交初始审查，以避免“阻止”审查位置。但提交部分审查或要求更多时间是可以的。只需保持沟通。
  • 审查和其他开发通信通常应通过 pgsql-hackers 邮件列表进行。
  • 将审查作为包含补丁的电子邮件的回复发送。尽量保持电子邮件线程完整。不要为审查创建新的线程，否则原始作者可能看不到它。

补丁审查通常会经历的阶段包括

提交审查（所需技能：补丁、英文理解）

  • 补丁是否采用具有上下文的补丁格式？（例如：上下文 diff 格式）
    • 只有显示更改的行而没有上下文的“普通”或“纯” diff 格式补丁是不可接受的。
    • 理想情况下，提交者应该根据哪种格式使提交者的补丁更易读，选择上下文（diff -c）格式或统一（diff -u）格式。
  • 它是否可以干净地应用于当前的 git master？
  • 它是否包含合理的测试、必要的文档补丁等？

可用性审查（所需技能：测试技巧、查找和阅读规范的能力）

阅读补丁应该做些什么，并考虑

  • 补丁是否真正实现了这一点？
  • 我们想要这样吗？
  • 我们已经有了吗？
  • 它是否遵循 SQL 规范或社区认可的行为？
  • 它是否包含 pg_dump 支持（如果适用）？
  • 是否存在危险？
  • 是否涵盖了所有方面？

功能测试（所需技能：补丁、配置、制作、将错误管道到日志）

应用补丁，编译并测试

  • 该功能是否按预期工作？
  • 作者是否没有考虑一些极端情况？
  • 是否存在任何断言失败或崩溃？
    • 审查应使用configure选项进行

--enable-cassert和--enable-debug已打开；请查看使用 git 工作以获取完整的示例。这些将有助于找到可能被遗漏的代码问题。但请注意，使用这些参数构建的 PostgreSQL 副本将比没有使用这些参数构建的副本慢得多。如果你正在处理与性能相关的内容，例如测试补丁是否会减慢任何速度，请确保在测试执行速度之前不使用这些标志构建。--enable-cassert 的一些（但不是全部）性能损失可以通过在服务器启动时将debug_assertions = false 放入你的 postgresql.conf 中来关闭。请查看开发者选项以了解有关该设置的更多详细信息；它在使用--enable-cassert 进行构建时默认为 true。另请注意，虽然--enable-debug 在使用gcc 构建时不会有任何性能损失，但它在使用大多数其他编译器时肯定会有性能损失。

性能审查（所需技能：计时性能的能力）

  • 补丁是否会减慢简单的测试？
  • 如果它声称可以提高性能，它是否真的提高了？
  • 它是否会减慢其他事情？

编码审查（所需技能：指南比较、移植性问题经验、简单的 C 阅读技能）

详细阅读对代码的更改，并考虑

  • 它是否遵循项目编码指南？
  • 是否存在移植性问题？
  • 它是否可以在 Windows/BSD 等上工作？
  • 注释是否足够且准确？
  • 它是否按预期正确地执行？
  • 它是否会产生编译器警告？
  • 你能让它崩溃吗？

架构审查（所需技能：整个 PostgreSQL 项目架构经验）

在整个项目的背景下考虑对代码的更改

  • 所有内容是否以与其他功能/模块一致的方式完成？
  • 是否存在可能导致问题的相互依赖关系？

审查审查

  • 审阅者是否涵盖了这种审阅者应该做的一切？
检索自 "https://wiki.postgresql.ac.cn/index.php?title=Reviewing_a_Patch&oldid=38472"