我经常听到一些朋友说很难给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内核项目文档[1]中的一些概念,来制定以下的PR标准。 (关于如何开始给wildfly贡献代码,黑客指导(the hacking guide)[2]依然是最好的文档)

Wildfly贡献和提交PR标准

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

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

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

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

3) 分解变更内容—​但也不要分的太多

直接引用[1],我百分之百同意:

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

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

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

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

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

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

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

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

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

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

5) 重视和响应审查意见

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

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

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

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

6) 不要气馁

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

7) 你也可以审查代码!

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

8) 针对重大重构

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

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