审阅者指南#
审查开放的拉取请求(PR)有助于推动项目前进.我们鼓励项目外的人也参与进来;这是熟悉代码库的一个好方法.
谁可以是审阅者?#
评论可以来自NumPy团队之外——我们欢迎领域专家(例如,`linalg`或`fft`)或其他项目维护者的贡献.您不需要成为NumPy维护者(有权限合并PR的NumPy团队成员)来进行审查.
如果我们还不认识你,建议在开始审查拉取请求之前,在 邮件列表或Slack 上介绍一下自己.
沟通指南#
每个PR,无论好坏,都是一种慷慨的行为.以积极的评论开始将帮助作者感到被奖励,而你随后的评论可能会被更清楚地听到.你也可能会感觉良好.
如果可能,从大的问题开始,这样作者就知道他们已经被理解了.抵制立即逐行进行或从小问题开始的习惯.
你是项目的门面,NumPy 在一段时间前决定 它将成为什么样的项目:开放、富有同情心、欢迎、友好和耐心.对贡献者要 友善.
不要让完美成为好的敌人,尤其是在文档方面.如果你发现自己提出了很多小建议,或者对风格或语法过于挑剔,考虑在所有重要问题得到解决后合并当前的PR.然后,如果你是维护者,可以直接推送一个提交,或者自己打开一个后续的PR.
如果你需要帮助撰写评审回复,可以查看一些 评审的标准回复.
审核者清单#
- 在所有条件下,预期行为是否清晰?需要注意的一些事项:
对于像空数组或 nan/inf 值这样的意外输入会发生什么?
轴或形状参数是否被测试为 int 或 tuples?
如果一个函数支持这些,是否会测试不寻常的 dtypes?
变量名是否应为了清晰性或一致性而改进?
应该添加评论,还是应该删除那些无用或多余的评论?
文档是否遵循 NumPy 指南?文档字符串是否正确格式化?
代码是否遵循 NumPy 的 风格指南?
如果你是维护者,并且从PR描述中不明显,请在合并消息中添加一个简短的解释,说明分支做了什么,如果关闭一个问题,还要添加”Closes gh-123”,其中123是问题编号.
对于代码更改,至少需要一名维护者(即拥有提交权限的人)审查并批准拉取请求.如果你是第一个审查并批准更改的人,请使用 GitHub 批准审查 工具将其标记为已批准.如果拉取请求很简单,例如它是一个明显正确的错误修复,可以直接合并.如果它更复杂或更改了公共 API,请至少开放几天,以便其他维护者有机会审查.
如果你是一个已经批准的PR的后续评审者,请使用与新PR相同的评审方法(专注于更大的问题,抵制只添加一些吹毛求疵的诱惑).如果你有提交权限并且认为不需要进一步评审,请合并PR.
对于维护者#
在合并PR之前,确保所有自动化的CI测试通过,并且 文档构建 没有任何错误.
如果出现合并冲突,请要求 PR 提交者 在主分支上变基.
对于添加新功能或以某种方式复杂的PR,请在合并之前至少等待一两天.这样,其他人就有机会在代码进入之前发表评论.考虑将其添加到发布说明中.
在合并贡献时,提交者负责确保这些贡献符合 NumPy 的 开发流程指南 中列出的要求.此外,检查新功能和向后兼容性破坏是否已在 numpy-discussion 邮件列表 上讨论过.
压缩提交或清理你认为过于混乱的PR的提交消息是可以的.记得在这样做时保留原作者的名字.确保提交消息遵循 NumPy 的规则.
当你想拒绝一个PR时:如果非常明显,你可以直接关闭它并解释原因.如果不是,那么最好先解释为什么你认为这个PR不适合包含在NumPy中,然后让第二个提交者评论或关闭.
如果 PR 提交者 6 个月内没有回应你的评论,请将该 PR 移至不活跃类别,并附上”inactive”标签.此时,维护者可以关闭该 PR.如果有兴趣完成所考虑的 PR,可以随时通过评论表明,无需等待 6 个月.
鼓励维护者在合并前只需要进行小的更改时(例如,修复代码风格或语法错误)完成PR.如果PR变得不活跃,维护者可能会进行更大的更改.记住,PR是贡献者和审阅者之间的合作,有时直接推送是完成它的最佳方式.
API 变化#
如前所述,大多数公共 API 的更改应事先讨论,并且通常与更广泛的受众(在邮件列表上,甚至通过 NEP)进行讨论.
对于公共 C-API 的更改,请注意 NumPy C-API 是向后兼容的,因此任何添加都必须与以前版本的 ABI 兼容.如果不是这种情况,您必须添加一个防护.
例如 PyUnicodeScalarObject
结构包含以下内容:
#if NPY_FEATURE_VERSION >= NPY_1_20_API_VERSION
char *buffer_fmt;
#endif
因为在NumPy 1.20中,``buffer_fmt`` 字段被添加到其末尾(所有之前的字段保持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 的 已保存的回复 中可能会有所帮助,以便于审阅:
- Usage question
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.org/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.
- Software versions
To help diagnose your issue, please paste the output of: ``` python -c 'import numpy; print(numpy.version.version)' ``` Thanks.
- Code blocks
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!
- Linking to code
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://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword).
- Regression test needed
Please add a [non-regression test](https://en.wikipedia.org/wiki/Non-regression_testing) that would fail at main but pass in this PR.
- Don’t change unrelated
Please do not change unrelated lines. It makes your contribution harder to review and may introduce merge conflicts to other pull requests.