审查代码

Sage中的所有代码都经过了同行评审。这有两个原因：

  • 因为开发人员不可能一下子想好所有事情

  • 因为一双新的眼睛可能会发现数学错误、代码中的角例、文档不足、遗漏一致性检查等。

任何人(例如你)都可以为别人的公关做这项工作。本文档列出了审查者在决定将公关纳入SAGE之前必须检查的内容。

现在，您可以通过阅读diff代码开始审查。

Check the GitHub checks: 我们要求所有的检查都通过了。

Read the diff: 点击PR的“Files Changed”标签。阅读所有修改过的文件的更改。我们用 pull request reviews 。您可以直接向更改的行添加注释。

Build the code: (这是可选的，如果 build and test 支票已通过。)当您阅读代码时，您可以 rebuild Sage with the new code 。如果您不知道如何 download the code ， see here 。

在阅读和测试代码时，通常应该检查以下内容：

  • The purpose ：该守则是否符合公关的既定目标？它会带来什么新的问题吗？使用各种输入测试新的或固定的功能，而不仅仅是文档中的示例，是否会产生预期的和健壮的输出(并且没有意外的错误或崩溃)？

  • User documentation ：新代码的使用对用户来说清楚吗？是否所有涉及的数学概念都是标准的，或者是否提供了解释(或链接)？如果他/她需要新代码，他/她能否轻松找到它？

  • Code documentation ：代码是否有足够的注释，以便开发人员不必想知道它到底做了什么？

  • Conventions ：代码尊重吗？ Sage's conventions ？ Python's conventions ？ Cython's conventions ？

  • Doctest coverage ：所有函数都包含doctest吗？使用 sage -coverage <files> 去检查一下。是否测试了新的/修改的方法和类的所有方面(请参见 编写可测试的示例 )?

  • Bugfixes ：如果PR包含错误修复，它是否会添加一个文档测试来说明该错误已修复？这个新的文档测试应该包含问题或公关号，例如 See :issue:`12345 `。

  • Speedup ：PR能让现有代码变得更慢吗？如果PR声称可以加快一些计算速度，那么PR是否包含代码示例来说明这一说法？公关应该解释加速是如何实现的。

  • Build the manuals ：参考手册构建时是否没有错误(同时检查html和pdf)？看见 《Sage手册》 学习如何制作手册。

  • Look at the manuals ：参考手册看起来还好吗？更改可能会出现打字错误，从而使文档构建时没有明显错误，但这可能会导致格式错误的输出或断开的超链接。

  • Run the tests ：所有文档测试都通过了，没有错误吗？Sage的不相关组件可能会受到此更改的影响。检查整个库中的所有测试，包括“long”文档测试(这可以用 make ptestlong )以及与该功能相关的任何可选文档测试。看见 运行Sage‘s Doctest 以获取更多信息。

您现在已准备好更改PR的状态(请参见 公关的地位 )：

  • positive review ：如果上述问题和其他合理问题的答案是 "yes" ，您可以将PR设置为 positive review 状态。

  • needs work ：如果有些事情不是应该的，在评论中列出所有需要解决的点，并将PR的状态更改为 needs work 状态。

  • needs info ：如果您不清楚并阻止您继续进行审查，请提出您的问题，并将PR的状态设置为 needs info 状态。

  • 如果你 do not know what to do 例如，如果你觉得没有足够的经验来做出最终决定，可以在评论中解释你已经做了什么，并询问其他人是否可以看看。

有关复习的更多建议，请参阅 How to Referee Sage Trac Tickets (注意：Git取代了Mercurial，GitHub取代了Trac)。

备注

“完美是善的敌人”

审查的重点是确保遵守Sage规范指南，并确保实施在数学上是正确的。请避免额外的功能请求或关于替代实现的开放式讨论。如果您希望以不同的方式编写代码，您的建议应该是一个明确且可操作的请求。

审核和关闭请购单

当他们有正面的评价或其他原因时，他们可以关闭。

如果PR因正面评价以外的其他原因关闭，请使用 resolution 标签 r: duplicate ， r: invalid ， r: wontfix ，以及 r: worksforme 。添加一条评论，解释为什么这个问题已经结束，如果这一点从讨论中还不清楚。

如果您认为某个问题已过早结束，请随时重新打开它。

使请愿书无效的理由

One Issue Per One Issue ：一个问题只能涉及一个问题，而不应该是一长串不相关的问题。如果一个问题涵盖多个问题，我们无法关闭它，虽然一些补丁已经应用到给定的版本，但问题仍将处于悬而未决的状态。

No Patch Bombs ：进入Sage的代码要经过同行审查。如果您出现了80,000行代码包，这些代码包完全摧毁了一个子系统，并用其他东西取代了它，您可以想象审查过程将会有一点乏味。这些巨大的补丁炸弹有几个原因是有问题的，我们更喜欢容易审查和应用的小而渐进的变化。这并不总是可能的(例如强制重写)，但仍强烈建议您避免这种开发风格，除非别无选择。

Sage Specific ：Sage的理念是我们将所有东西(或接近它的东西)放在一个源代码tarball中，以使调试成为可能。您可以想象，如果您只用外部包替换Sage的10个组件，我们将不得不处理的组合爆炸。一旦您开始更换通常打包的Sage的一些更基本的组件(例如，Pari、GAP、LISP、GMP)，这就不再是我们跟踪器中的问题。例如，如果您的发行版的Pari包有错误，请向他们提交错误报告。我们通常愿意也有能力解决问题，但不能保证我们会帮助你解决问题。看看特定于Sage的PR的公开数量，您希望能理解其中的原因。

No Support Discussions ：GitHub不是一个在使用Sage时跟踪问题的系统。问题应该是一个明显的错误，而不是“我试着做X，但我不能让它工作。我该怎么做？”这在Sage中通常不是一个错误，很可能 sage-support 我可以为你回答这个问题。如果事实证明你确实遇到了漏洞，有人会打开一份简明而直截了当的公关。

Solution Must Be Achievable ：问题必须是可以实现的。很多时候，属于这一类的问题通常与上面列出的一些其他规则相冲突。一个例子就是“让圣贤成为世界上最好的中科院”。没有合适的衡量标准来衡量这一点，而且它是高度主观的。

发布过程

对于开发人员和审阅人员来说，了解Sage Release Manager用于发布的过程是有好处的。以下是截至2024年的数据：

Beta Release Stage ：对于准备新的测试版或第一个候选版本，所有具有即将发布里程碑的正面评价的PR都将被考虑。发布经理分批合并10到20个PR。如果PR与发布经理的分支发生合并冲突，则发布经理会将PR设置回“Need Work”状态。(PR的作者可以尝试猜测哪些其他PR可能导致冲突，进行合并提交并将它们声明为依赖项，然后将其设置回“积极审查”状态。或者，公关作者可以等到下一个测试版发布，然后解决冲突。)每一批合并的PR都要进行集成测试。如果检测到问题，请购单将被设置回“需要工作”状态并取消合并。当一批请购单准备好时，发放经理关闭这些请购单并继续下一批。在几个批次之后，新的测试版被标记，并被推送到 develop GitHub上的Sage存储库分支，并于 sage-release 。

Release Candidate Stage ：在确定了第一个发布候选版本后，项目处于发布候选版本阶段，并使用修改后的程序。现在 only PRs with a priority set to "blocker" are considered 。所有其他优先事项，包括“危急”，都会被忽略。因此，如果票据足够重要，应包含在此阶段，则应将其设置为“拦截器”，方法是添加 p: blocker / 1 标签。

Blocker PRs ：发布过程的目标是高质量的稳定发布。请注意，合并公关需要权衡风险和收益。合并公关的好处是公关带来的改进，比如修复错误。然而，任何代码更改都有引入不可预见的新问题并因此延迟发布的风险：如果一个新问题引发另一个候选发布，它可能会将发布推迟至多2周。因此，开发人员应谨慎使用“拦截器”优先级，并应说明PR的基本原理。尽管没有一个固定的规则或权威机构来确定什么是适合“拦截者”状态的，

  • 引入新功能的PR通常不会成为障碍--除非它们可能完善了本发布周期开发的一组功能。

  • 对代码进行重大更改的PR，例如重构PR，通常不是拦截器。

Final Release ：如果最后一个候选版本没有拦截器PR，则发布经理将其转换为最终版本。它被标记为发布里程碑，并于 sage-release 。

发布管理脚本保存在存储库中 sagemath/sage-release-management 。