# Pull Requests指南 本文介绍了向 [DragonOS项目](https://github.com/DragonOS-Community/DragonOS) 及其子项目提交Pull Requests的流程。 *如果您尚不熟悉我们的GitHub工作流程，请先阅读[GitHub工作流程]* [[toc]] ## 在提交PR之前 本指南适用于已经在本地完成开发的人。如果您想知道如何设置开发环境，以及如何为DragonOS社区作出贡献，请转到[贡献者指南] 如果您是第一次为DragonOS社区作出贡献，请先从’[为DragonOS社区作出贡献]’这篇文章开始阅读！ **确保您的拉取请求符合我们的最佳实践。这些实践包括遵循项目规范、提交小的拉取请求以及充分注释。** ### 运行本地的测试 当你在提交PR之前，请运行以下命令进行本地测试： * 格式化代码（`make fmt`或者`cargo fmt`） * 运行静态测试 （对于DragonOS仓库而言，是`cd kernel && make test`） ## PR的流程 - [创建一个PR](https://help.github.com/articles/about-pull-requests/) - 签署CLA（暂时还不需要） - 通过自动化测试 - 获得来自reviewer，以及代码拥有者的“Approve” ## 某个Feature或者PR是必要的吗？ 有时候我们想到了一些点子，或者发现了一些“BUG”（请注意是有双引号的）。在开始开发之前，最好的方式是先与社区的成员们讨论它！ 也许这些feature是不必要的，或者不是bug。在讨论后，你将能获得更好的解决方案！ 您可以在[社区沟通渠道]上发帖，与大家交流！ ## 保持简洁、无需、最小可行性产品等原则 有时我们需要提醒彼此软件设计的核心原则——“保持简洁”、“你不需要它（YAGNI）”、“最小可行性产品（MVP）”等。 往项目里面添加“也许我们以后需要（但现在用不上，也难以充分验证）”的功能，是与我们的开发原则相违背的。 请你添加你现在需要的功能，并且为将来可能需要的功能，留出可扩展的空间——但是请不要现在实现他们！ ## 小即是美：小的提交，小的请求 小的提交和小的请求审查得更快，而且比大的更可能正确。 *注意力是一种稀缺资源。* 如果你的请求需要60分钟来审查，审查者在最后30分钟的注意力可能不如最初的30分钟敏锐。 如果需要一个大的连续时间段来进行审查，它可能根本就不会被审查！ ### 拆分commit 将你的请求在每个逻辑断点处进行拆分，分成多个commits. 多个逻辑上有意义的提交，有助于向审查者展示你的开发过程，以及你的想法。 **请努力将逻辑上不同的想法分组到单独的提交中。** 例如，如果你发现Feature-X需要一些重构才能适应，就做一个只进行那种重构的提交， 然后再为Feature-X做一个新的提交。 在提交数量上要找到一个平衡点。 一个包含25个提交的请求仍然很难审查，所以要运用你的最佳判断。 ### 拆分PR 或者，回到我们重构的例子，你也可以创建一个新分支，在那里进行重构，然后为那个功能发一个PR。 如果你能从你的PR中，把单独的，可合并到主线的部分作为单独的PR来提交到主线，你就可以避免不断rebase的痛苦问题。 DragonOS的仓库主线一直在更新，请你尽快提交小的PR、小的更改。（你的PR更早合并进主线的话，其他繁琐的事情就是别人的了） **多个小的请求通常比多个提交更好。** 不要担心太多的小PR会淹没我们。我们宁愿有100个小的、明显的PR，也不愿有10个无法审查的庞然大物。 我们希望每个PR都能独立工作，所以在拆分时，决定是作为独立的PR还是独立的commit时，要结合你的最佳判断。 作为一个经验法则，如果你的请求与Feature-X直接相关，而不涉及其他内容，它应该可能是Feature-X请求的一部分。 如果你能解释你为什么做看似无用的操作,比如：“我保证它使Feature-X的变更更容易”，我们可能就会接受你的PR。 ## 为修复和通用功能开启不同的PR **将与你正在开发的feature无关的更改放入不同的PR中。** 通常，在你实现Feature-X的过程中，你会发现差的注释、命名不当的函数、糟糕的结构、薄弱的类型安全等问题。 你绝对应该修复这些问题（或者至少请提出issue），但不要与你的特性放在同一个请求中。否则，你的差异将会有太多的变化，你的审查者将无法看到树木之外的森林。 **寻找将功能提取出来的机会。** 例如，如果你发现自己接触了很多模块，思考一下你在包之间引入的依赖关系。 你正在做的一些事情能否变得更加通用，并从Feature-X包中移出并上移？ 你是否需要使用一个与其它包无关的函数或类型？ 如果是，就把这些代码单独封装/提出来！ 同样，如果Feature-X与上个月提交的Feature-W在形式上相似，并且你正在复制Feature-W中的一些棘手内容，考虑将核心逻辑提取出来，并在Feature-W和Feature-X中都使用它。 （请在一个单独的commit或PR中这样做。） ## 不要开启涵盖整个仓库的PR 通常，一个新的贡献者会在仓库的许多地方发现一些问题，并提交一个PR一次性修复所有问题。 也许最新的Rust版本中有一个很酷的新函数，每个人都应该使用，或者有一个最近被弃用的函数，应该用调用其替代函数来替换。有时，贡献者会运行linter或安全扫描器来查找问题，或者在注释或变量名中修复特定的拼写错误。 这种方法的问题是，仓库的不同部分由不同的SIG维护，因此对这些不同部分的更改需要得到不同人的批准。一个包含20个单行更改、散布在仓库各处的PR可能最终需要5个或10个以上的批准才能合并。（尽管有少数人可以批准对仓库大部分内容的更改，但这些人通常是最忙碌、最难获得审查的，特别是当你是一个在社区内还没有联系的新贡献者时。） 如果你真的想尝试让这样的PR合并，你最好的做法是将PR分解，把他拆分为每个它所涉及的SIG的单独PR。你可以查看仓库下的`triagebot.toml`文件，看看谁拥有那段代码，然后相应地将你的修改分为多个PR。 ## 注释很重要 在你的代码中，如果有人可能不理解你为什么这么做（或者你以后会忘记为什么），那就加上注释。许多代码审查的评论都是关于这个确切的问题。 如果你认为有一些非常明显的事情我们可以跟进，请添加一个TODO。 阅读[代码风格指南]- 遵循这些关于注释的一般规则。 ## 测试 没有什么比开始审查时发现测试不充分或缺失更令人沮丧的了。 **如果你的PR测试不充分，或者压根没有测试，审查者会很恼火，或者根本不会审查你的代码！** 非常少的PR可以在不触及测试的情况下修改代码。 **如果你不知道如何测试Feature-X，请提问！** 我们将很高兴帮助你设计易于测试的东西，或者建议合适的测试用例。 ## Commit Message指南 **请注意，PR的标题应该遵守“[约定式提交](#使用“约定式提交”)”的规范** PR评论不会出现在提交历史中。 提交和它们的提交信息是您的PR中正在进行的更改的_“永久记录”_，它们的提交信息应该准确地描述_什么_和_为什么_。 提交信息由两部分组成：主题和正文。 主题是提交信息的首行，对于小或微不足道的更改通常只需要这部分。 这些可以通过`git commit -m`或`--message`标志作为“单行”完成，但只有在可以用这几句话完全描述这个commit时，才可以这样做。 提交信息的正文是当你运行`git commit`不带`-m`标志时，主题下方的文本部分，这将在你的编辑器中打开提交信息进行编辑。 输入一些解释性的句子对于你的审查和整体后期的项目维护是非常有用的。 ``` 提交类型：这是提交信息的主题 这里的任何文本都是提交信息的正文 一些文本 更多的文本 ... # 请输入你的更改的提交信息。以'#'开头的行将被忽略，空消息会终止提交。 # # 在分支 example # 要提交的更改： # ... # ``` 使用以下指南来帮助编写格式良好的提交信息。 - [使用“约定式提交”](#使用“约定式提交”) - [尝试将主题行保持在50个字符以内；不要超过72个字符](#尝试将主题行保持在50个字符以内-不要超过72个字符) - [提交信息主题的第一个词应该大写，除非它以小写符号或其他标识符开头](#提交信息主题的第一个词应该大写-除非它以小写符号或其他标识符开头) - [不要以句号结束提交信息主题](#不要以句号结束提交信息主题) - [在提交信息主题中使用祈使语气](#在提交信息主题中使用祈使语气) - [在提交信息正文前添加一个空白行](#在提交信息正文前添加一个空白行) - [不要在提交信息中使用GitHub关键词或(@)提及](#不要在提交信息中使用github关键词或-提及) - [使用提交信息正文来解释提交的_什么_和_为什么_](#使用提交信息正文来解释提交的_什么_和_为什么_) ### 使用“约定式提交”   关于约定式提交的详细规范，在[Conventional Commit/约定式提交](https://www.conventionalcommits.org/zh-hans/v1.0.0/)网站中有详细介绍，在本节末尾将给出示例(摘自[Conventional Commit/约定式提交](https://www.conventionalcommits.org/zh-hans/v1.0.0/)网站)，可选择性阅读。我们做出以下特别说明： 1. 由于DragonOS内核原则上仅通过系统调用接口保证对外可用性，而迄今为止(2024/04/22)，出于对软件生态的考量，DragonOS选择实现与Linux一致的系统调用，因此不会对`破坏性变更(BREAKING CHANGES)`做特殊说明，或者说，在当前开发环境中不会产生对用户产生显著影响的破坏性变更，因此无特殊需要，DragonOS内核不应使用诸如`feat!`来表示破坏性变更。(内核之外，例如dadk，仍需遵循规范) 2. DragonOS社区严格遵循基于squash的工作流，因此我们并不强制要求PR中的每一个单独的commit都符合[Conventional Commit/约定式提交](https://www.conventionalcommits.org/zh-hans/v1.0.0/)，但是我们仍强烈建议使用。 3. 关于scope: 如无特殊说明，以子模块/系统/目录名作为范围，例如代码变动是发生在`kernel/src/driver/net`中的特性追加，那么应当命名为`feat(driver/net):`，如果是发生在`kernel/src/mm/allocator`中，应当命名为`feat(mm)`，简而言之就是要尽可能简短的表现出所属模块，大多数情况下，不应使用超过两级的范围标识，例如`fix(x86_64/driver/apic)`是错误的，应当命名为`fix(x86_64/apic)` 4. 在DragonOS内核代码仓库中的`issue checker`会对标题格式进行简单审查，如果不符合格式的将会被标记为`ambiguous`，贡献者们请按需修改 5. 使用小写 ### 尝试将主题行保持在50个字符以内；不要超过72个字符 50字符的主题行限制是为了保持消息摘要尽可能简洁。 它应该只是足以描述正在做什么。 72个字符的硬限制是为了与最大正文大小对齐。 当使用`git log`查看仓库历史时，git会使用额外的空格填充正文文本。 将宽度 控制 在72个字符可以确保正文文本在80列终端中居中并易于查看。 #### 提供额外上下文 你可以使用更少的字符通过在提交信息前加上你PR影响的[类型]或[区域]来提供额外上下文。 这些标签通常是DragonOS社区其他成员会理解的常用标签。 **例如：** - `feat(mm/allocator): 添加xxxx分配器` - `feat(api)!: send an email to the customer when a product is shipped` - `fix(epoll): 修复epoll在xxx场景下无法唤醒目标进程的问题` 这些可以在正文进一步阐述这个PR或者这个commit的内容。 ### 提交信息主题的第一个词应该大写，除非它以小写符号或其他标识符开头 提交信息主题就像一个缩写句子。 第一个词应该大写，除非消息以符号、首字母缩略词或其他标识符开头，这些通常应该是小写的。 ### 不要以句号结束提交信息主题 这主要是为了节省空间，也有助于推动主题行尽可能短和简洁。 ### 在提交信息主题中使用祈使语气 祈使语气可以被认为是_“下达命令”_；它是一个**现在时态**的陈述，明确描述正在做什么。 **好的例子：** - 修复y中的x错误 - 将foo添加到bar - 撤销提交“baz” - 更新PR指南 **不好的例子** - 修复了y中的x错误 - 添加了foo到bar - 撤销了错误的提交“baz” - 更新了PR指南 - 修复更多问题 关于形成祈使语气的提交主题的一般准则是，它应该能填充这个句子： ``` 如果应用，这个提交将 <你的主题行在这里> ``` **例子：** - _如果应用，这个提交将_ **修复y中的x错误** - _如果应用，这个提交将_ **将foo添加到bar** - _如果应用，这个提交将_ **撤销提交“baz”** - _如果应用，这个提交将_ **更新PR指南** ### 在提交信息正文前添加一个空白行 Git使用空白行来确定提交信息的哪部分是主题和正文。 主题前的文本是主题，而主题后的文本被认为是正文。 ### 将提交信息正文 wrap 在72个字符 Git的默认列宽是80字符。 当查看git日志时，git会在正文文本周围添加额外的空格。 这将为您留下76个可用字符的空间，但是文本将显得“偏斜”。 为了使文本在80列终端中居中并易于查看，Git会使用相同的空格量在另一侧进行人工填充，结果在每行上可用72个字符。 想象一下它们是word文档的边距。 ### 不要在提交信息中使用GitHub关键词或(@)提及 #### GitHub关键词 在提交信息中使用[GitHub关键词]后跟`#<问题编号>`引用将自动在您的PR上应用`do-not-merge/invalid-commit-message`标签，从而阻止它被合并。 [GitHub关键词]在PR中用于关闭问题是一种便利，但当在提交信息中使用时，可能会产生意外的副作用；通常关闭了它们不应该关闭的东西。 **被阻止的关键词：** - close - closes - closed - fix - fixes - fixed - resolve - resolves - resolved #### (@)提及 在提交信息中的(@)提及将向该用户发送通知，并且每当PR更新时，都会持续发送。 ### 使用提交信息正文来解释提交的_什么_和_为什么_ 提交和它们的提交信息是您的PR中正在进行的更改的_“永久记录”_。 描述为什么事情发生了变化以及它可能产生的影响。 你为审查者和下一个需要接触你代码的人提供了上下文。 如果某个东西正在解决一个bug，或者是对某个特定问题的响应，你可以在正文本身中链接它作为参考。 这些类型的面包屑对于追踪未来的bug或回归至关重要，并进一步帮助解释为什么这个提交被创建。 **其他资源：** - [如何编写Git提交信息 - Chris Beams](https://chris.beams.io/posts/git-commit/) - [分布式Git - 贡献给项目（提交指南）](https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project) - [50/72规则是怎么回事？ - Preslav Rachev](https://preslav.me/2015/02/21/what-s-with-the-50-72-rule/) - [关于Git提交信息的笔记 - Tim Pope](https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html) ## 适当的回击是可以的 有时候审查者会犯错。如果你有充分的理由坚持自己的做法，那么你有权与审查者讨论所请求更改的优点。审查者和被审查者都应该以礼貌和尊重的方式讨论这些问题。 你可能会被推翻，但你也可能会成功。我们都是一些相当理性的人。 开源项目的另一个现象（任何人都可以对任何问题发表评论）是“狗群效应”——你的PR收到了来自众多人的众多评论，使得跟进变得困难。在这种情况下，你可以询问主要审查者（分配者）是否希望你创建一个新的分支来清理所有评论。你不必修复每一个评论者提出的所有问题，但应该对合理的评论给出解释。 ## 琐碎的编辑 每个进来的PR都需要被审查、检查，然后合并。 尽管自动化可以帮助处理这一点，但每个贡献也都有工程成本。因此，如果你发现文件中有一个语法错误或拼写错误，很可能该文件中还有更多类似的错误，你可以通过检查格式、检查链接是否有效以及修复错误，然后一次性提交所有修复，从而使你的请求发挥最大的作用。 **一些需要考虑的问题：** * 文件是否还可以进一步改进？ * 这次琐碎的编辑是否大大提高了内容的质量？ ## 工作流 DragonOS社区使用`@dragonosbot`来执行自动化的工作流。 以下是一个PR从创建到合并的过程 1. 创建PR 1. `@dragonosbot assign` 来分配审查者 1. Reviewer审查了你的代码，并且提出了意见。接着，reviewer需要在审查意见的末尾添加一行`@dragonosbot author`，告知机器人把PR的状态更改为“等待作者修改”。 1. 把你的更改推送到当前分支 1. 添加评论，描述你的更改，并且在评论的最后一行添加`@dragonosbot ready` 1. 重复上述步骤直到审查无问题后，具有合并权限的审查者会将您的代码合并到主线。 [GitHub工作流程]: /contributors/github-workflow.md [贡献者指南]: /contributors/README.md [为DragonOS社区作出贡献]: /contributors/README.md [社区沟通渠道]: /communication/README.md [代码风格指南]: /contributors/code-style.md