如何给Wildfly提交PR

| July 18, 2015

我经常听到一些朋友说很难给JBoss开源社区提交代码。

事实上的确如此，带有redhat或者jboss的邮件地址的贡献者会有优势，但代码质量和补丁描述才是最关键的。

开源社区也有不同的文化，JBoss属于相对紧凑型的，简单来说开源贡献者需要适应JBoss的一些约定和规则。

这篇文章是前几天David M.Lloyd发到wildfly邮件列表的。原文在此 http://lists.jboss.org/pipermail/wildfly-dev/2015-July/004243.html

David目前是wildfly的总架构师，可以说从AS7以后的JBoss应用服务器产品的基本架构是他奠定的。

David在多个项目上都体现出超凡的能力，如JBossRemoting, JBossLogging, XNIO, MSC, JBossModule等等。

这篇文章是难得的好文，指点如何给JBoss社区提交PR，包含一些规则和窍门。我把它翻译过来供大家参考。同时也是给其他开源社区贡献的参考资料。

如何给Wildfly提交PR(PullRequest)

自从代码管理迁移到Git以来，以及采用了审查为导向的贡献体系(contribution structure)，对于wildfly和相关的开源项目，我们都已经看到了巨大的提高，包括质量和数量方面。然而，尽管我们已经写了很多文档，包括如何获取源码，进行修改，使用git创建分支，提交PR等等。但还没有讨论过如何比较标准的进行实际构建(actually build)和结构化PR请求(structure pull requests)，这就是写这篇文章的目的。

有几个做这件事的原因：

首先，审查者人数现在很少。这样有几个不好的效果：

  • PR在审查队列中周期很长
  • 巨型PR获得的审查机会比小的PR还少
  • PR得到的审查质量及其不稳定

其次，我们看到各种有问题的RP请求，比如单一大文件无法审查的PR，成千小文件PR，混合了表单和函数各种功能改进的PR，还有一些隐蔽的情况，你如提交之间让编译失败的PR。

第三，项目已经发展到了一定的规模和范围，我们需要有更多的人手盯紧每一个变化-实际只能做到尽可能多的。

为此，借用Linux内核项目文档中的一些概念，来制定以下的PR标准。 (关于如何开始给wildfly贡献代码，黑客指导(the hacking guide)依然是最好的文档)

Wildfly贡献和提交PR标准

我们为wildfly和相关子项目，制定一些重要git提交的规则和准则。但不是完整的git教程，因为那样会超出本文的范围。

  1. 充分的描述PR请求内容 描述应当包括JIRA编号，从而可以找到相关的相关的问题描述。描述语句恰当的，用人类易读的语言总结修改的内容。正确的拼写和语法是加分项。

  2. 首先要保证编译和测试通过

当审查者花了大量时间去审查代码，发现其甚至不能编译时，他们会很恼火。经常会看到提交patch来通过checkstyle代码格式检验。 有时我们会依赖自动化的CI/GitHub功能来进行编译和测试。但这样通常会带来麻烦，所以不要这样做！

  1. 分解变更内容–但也不要分的太多

直接引用1，我百分之百同意：

“将每个变化逻辑(logical change)划分为一个单独的补丁

“例如，如果你的改动是为一个驱动程序所写，包括bug修复和性能增强，那么划分为两个或者以上的补丁。如果改动包括API更新，及使用新API的驱动，划分为两个补丁。

“另一方面，如果你一次修改动了大量的文件，那么把这些改动划分在一起，这样一个补丁中包含逻辑变化所有修改。

“关键要记住，每个补丁应当让审查者很容易理解和验证。每个补丁的作用正当有理。

“一个补丁依赖于另一个补丁来保证完整性是可以的，需要在提交时注明‘这个补丁依赖于补丁X’。

“当你划分修改为一系列补丁，要特别注意确保wildfly可以在每个补丁后可以正确的编译和运行。有时开发者使用’git bisect’来追查问题，到你的一系列提交时没法继续时，他们可能会骂娘。

我想强调区分“功能”和“非功能”的改变非常重要，后一类包括格式化工作（建议没有必要的原因就不要做）

  1. 避免大规模的分支和/或意识流"stream of consciousness"分支

我们知道开发是一个反复迭代的过程。尽管如此，我们并不需要记录完整的历史（比如在一次PR中先有"add foobar"紧跟着"remove foobar"）。特别是对于大的变化或者大的项目（如wildfly）。一个好的实践是提交者重新审视提交，重新排列和结构化提交改变，只包含对于主干增量变化。

如果PR包含上百个普通提交，那么强烈建议分割成多个PR，每个PR的大小方便进行高效审查。如果太大了，审查者要么是没有进行足够的审查就匆忙合并，要么就忽略掉甚至直接关掉。你可以设想哪种情况更糟糕。

  1. 重视和响应审查意见

虽然是我的经验，但wildfly贡献者都赞成这点。我直接引用1的内容

“你的补丁几乎肯定能得到来自审查者的意见，告知如何改善，你比如回应这些意见，忽略意见的结果一般就是PR被忽略。

“一定要告诉审查者你做的修改，并感谢他们投入的时间。代码审查是一项累人费时的过程，审查者有时会脾气暴躁。即便如此，也要礼貌和解决他们指出的问题”

此外，当需要进行修改时，正确的方式是修改原始提交，而不是在其之上进行添加。参考(4)内容。

  1. 不要气馁

在PR接受前，多次迭代提交是很正常的，它总有机会通过审查。不要气馁–反而这是好的现象，说明审查者高度关注代码库质量。同时，也要考虑审查者已经一遍遍反复说了同样提交意见，所以也请关怀他们，尽可能高质量提交，参考(5)。

  1. 你也可以审查代码！

你不必成为正式的审查者，也可以审查PR。如果看到熟悉的领域，尽管去检验和进行注解来帮助评审。另外，所有的PR都需要进行基本审查保证非机器检验的正确性，如注意到糟糕的代码，NPE风险和运用反模式，还有无聊的拼写，语法和文档。

  1. 针对重大重构

当遇到重大和长期重构时，上述限制变得不切实际，尤其是划分变化。这时，你可以使用一个工作分支来应用上述的规则。同时可以也提交给审查者。周期性的一段时间后，合并提交到上游分支上。

运用这种方式，当长期分支准备合并到主干“回家”时，审查者已经对大多数改变进行过评审，从而更容易通过。

搜索

分类

标签