审阅者指南#
审查开放的拉取请求 (PR) 有助于推动项目向前发展。我们鼓励项目外部的人员也参与进来;这是一种熟悉代码库的好方法。
谁可以成为审阅者?#
审查可以来自 NumPy 团队之外——我们欢迎领域专家的贡献(例如,linalg 或 fft)或其他项目的维护者。您无需成为 NumPy 维护者(拥有合并 PR 权限的 NumPy 团队成员)即可进行审查。
如果我们还不认识您,请在开始审查拉取请求之前考虑在邮件列表或 Slack 上进行自我介绍。
沟通指南#
每个 PR,无论好坏,都是一种慷慨的行为。以积极的评论开头将帮助作者感到欣慰,并且您的后续评论可能会被更清楚地听到。您也可能感觉良好。
如果可能,从大型问题开始,以便作者知道您已经理解了问题。抵制立即逐行检查或以小的普遍问题开头的诱惑。
您是项目的代言人,NumPy 很久以前就决定了它将成为哪种类型的项目:开放、富有同理心、欢迎、友好和耐心。对贡献者友善。
不要让完美成为好的敌人,尤其是在文档方面。如果您发现自己提出了许多小的建议,或者对样式或语法过于吹毛求疵,请考虑在解决所有重要问题后合并当前的 PR。然后,您可以直接推送提交(如果您是维护者)或自己打开后续 PR。
如果您需要帮助撰写审查回复,请查看一些审查标准回复。
审阅者检查清单#
- 在所有条件下,预期行为是否清晰?一些需要注意的事项
空数组或 nan/inf 值等意外输入会发生什么?
是否测试轴或形状参数为int 或tuples?
如果函数支持,是否测试了不寻常的dtypes?
是否应改进变量名称以提高清晰度或一致性?
是否应添加注释,或者将其删除因为它们无用或多余?
文档是否遵循NumPy 指南?文档字符串是否正确格式化?
代码是否遵循 NumPy 的样式指南?
如果您是维护者,并且 PR 说明中没有明确说明,请在合并消息中添加对分支所做操作的简短说明,如果关闭了问题,则还添加“Closes gh-123”,其中 123 是问题编号。
对于代码更改,至少需要一位维护者(即拥有提交权限的人员)审查并批准拉取请求。如果您是第一个审查 PR 并批准更改的人,请使用 GitHub 的批准审查工具将其标记为已批准。如果 PR 很简单,例如它是明显的正确错误修复,则可以立即合并。如果它更复杂或更改了公共 API,请将其保持打开状态至少几天,以便其他维护者有机会审查。
如果您是已批准 PR 的后续审阅者,请使用与新 PR 相同的审查方法(重点关注较大的问题,抵制仅添加少量挑剔的诱惑)。如果您拥有提交权限并且认为不再需要审查,请合并 PR。
面向维护者#
在合并 PR 之前,请确保所有自动 CI 测试都已通过,并且文档构建没有任何错误。
如果发生合并冲突,请要求 PR 提交者重新设置到主分支。
对于添加新功能或以某种方式复杂的 PR,请至少等待一两天再合并它。这样,其他人就可以在代码加入之前发表评论。考虑将其添加到发行说明中。
合并贡献时,提交者有责任确保这些贡献符合 NumPy 的开发流程指南中概述的要求。此外,请检查是否已在numpy-discussion 邮件列表上讨论了新功能和向后兼容性中断。
压缩提交或清理您认为过于混乱的 PR 的提交消息是可以的。请记住,在执行此操作时要保留原始作者的姓名。确保提交消息遵循NumPy 规则。
当您要拒绝 PR 时:如果非常明显,您可以直接关闭它并解释原因。如果不是,则最好先解释为什么您认为 PR 不适合包含在 NumPy 中,然后让第二个提交者评论或关闭。
如果 PR 提交者在 6 个月内未回复您的评论,请将相关 PR 移动到“inactive”类别,并附加“inactive”标签。此时,维护者可以关闭 PR。如果有人有兴趣完成正在考虑的 PR,则可以在任何时候通过评论表示,无需等待 6 个月。
鼓励维护者在合并前仅需少量更改即可完成 PR(例如,修复代码样式或语法错误)。如果 PR 变为 inactive,维护者可以进行更大的更改。请记住,PR 是贡献者和审阅者/审阅者之间的合作,有时直接推送是完成它的最佳方式。
API 更改#
如前所述,大多数公共 API 更改应提前讨论,并且通常应与更广泛的受众讨论(在邮件列表上,甚至通过 NEP)。
对于公共 C-API 中的更改,请注意 NumPy C-API 向后兼容,因此任何添加都必须与先前版本 ABI 兼容。如果不是这种情况,则必须添加一个保护措施。
例如,PyUnicodeScalarObject
结构包含以下内容
#if NPY_FEATURE_VERSION >= NPY_1_20_API_VERSION
char *buffer_fmt;
#endif
因为buffer_fmt
字段是在 NumPy 1.20 中添加到其末尾的(所有先前的字段保持 ABI 兼容)。类似地,添加到 numpy/_core/code_generators/numpy_api.py
中的 API 表中的任何函数都必须使用 MinVersion
注释。例如
'PyDataMem_SetHandler': (304, MinVersion("1.22")),
仅限头文件的函数(例如新的宏)通常不需要进行保护。
GitHub 工作流程#
审查拉取请求时,请根据需要使用 GitHub 上的工作流程跟踪功能
审查完成后,如果您想要求提交者进行更改,请将您的审查状态更改为“请求更改”。这可以在 GitHub 上的 PR 页面、“已更改文件”选项卡、“审查更改”(右上角的按钮)上完成。
如果您对当前状态感到满意,请将拉取请求标记为已批准(与“请求更改”相同)。或者(对于维护者):如果您认为它已准备好合并,请合并拉取请求。
将拉取请求代码的副本检出到您自己的机器上可能会有所帮助,以便您可以本地使用它。您可以使用GitHub CLI通过单击 PR 页面右上角的Open with
按钮来执行此操作。
假设您已设置了开发环境,您现在可以构建代码并对其进行测试。
审查标准回复#
将其中一些存储在 GitHub 的已保存回复中以供审查可能会有所帮助
- 用法问题
You are asking a usage question. The issue tracker is for bugs and new features. I'm going to close this issue, feel free to ask for help via our [help channels](https://numpy.com.cn/gethelp/).
- 欢迎您更新文档
Please feel free to offer a pull request updating the documentation if you feel it could be improved.
- 错误的自包含示例
Please provide a [self-contained example code](https://stackoverflow.com/help/mcve), including imports and data (if possible), so that other contributors can just run it and reproduce your issue. Ideally your example code should be minimal.
- 软件版本
To help diagnose your issue, please paste the output of: ``` python -c 'import numpy; print(numpy.version.version)' ``` Thanks.
- 代码块
Readability can be greatly improved if you [format](https://help.github.com/articles/creating-and-highlighting-code-blocks/) your code snippets and complete error messages appropriately. You can edit your issue descriptions and comments at any time to improve readability. This helps maintainers a lot. Thanks!
- 链接到代码
For clarity's sake, you can link to code like [this](https://help.github.com/articles/creating-a-permanent-link-to-a-code-snippet/).
- 更好的描述和标题
Please make the title of the PR more descriptive. The title will become the commit message when this is merged. You should state what issue (or PR) it fixes/resolves in the description using the syntax described [here](https://githubdocs.cn/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword).
- 需要回归测试
Please add a [non-regression test](https://en.wikipedia.org/wiki/Non-regression_testing) that would fail at main but pass in this PR.
- 不要更改不相关的内容
Please do not change unrelated lines. It makes your contribution harder to review and may introduce merge conflicts to other pull requests.