In the Free and Open Source software worlds, it seems that a lot of people with good intentions don’t understand how to contribute efficiently and effectively to upstream software projects. I say this based on personal experience: I sometimes receive patches to projects I maintain, where despite good intentions and a perfectly valid goal (e.g. a bug-fix or feature implementation), the patches are fundamentally flawed in a variety of ways. So unfortunately I cannot accept them without negatively impacting the quality or maintainability of the project.  And I don’t even maintain any wildly popular projects.

It is possible to spend a lot of money on training to learn how to avoid these mistakes, and also many well-organized projects already provide good documentation on how to contribute, such as the Linux kernel and git. However, these documents typically mix up generic advice with project-specific advice. In this post I would like to present a list of generic advice on how to write patches which have the maximum chance of being accepted quickly and with gratitude rather than resentment!

1. Be sure that your contributions will be welcomed

Before starting work on a contribution, check that you are not reinventing the wheel! The feature or bug you want to work on may already be at least partially solved, so the first thing you should do is check the project’s bug/issue tracker and mailing list or other communication channel for prior work in the area.

If the project has a known bug and noone else has done any publically visible work on it, it’s very likely that a good fix for it will be welcomed. However, if you want to contribute a new feature, it might be worth checking whether it is likely to be accepted before you spend any significant effort working on it, since features can sometimes conflict with the project’s existing architecture or design patterns / goals. Unfortunately it’s not really possible to give specific hints on this in the general context of this article, but if your patches significantly revamp any aspect of the project, you should probably have discussed this with upstream and obtained approval before getting too far into the revamp.

2. Stay consistent with the existing code

Check your patch for stylistic inconsistencies with the upstream project.  This includes whitespace errors, e.g.

• using hard tabs for indentation if the upstream project uses spaces, or vice-versa
• using the wrong number of indentation columns
• lines with trailing space
• space before or after parentheses

and countless other stylistic issues.  The general rule is: be consistent, even if you don’t like the upstream style, because inconsistencies damage the project’s maintainability.  Make sure you have read some style guide documents for the languages you are using – there are plenty of good ones on the internet.

3. Submit patches in the way upstream wants to receive them

This is all about using the same tools and processes which the project’s maintainer(s) are used to using, not the ones which are most convenient for you.  If you choose to ignore upstream expectations, you are asking the maintainer(s) to spend extra time adapting to your particular habits, which is akin to implying that you consider the maintainer’s time to be less valuable than your own. This is not very polite, so look to see if the project has documentation in its code base or website explaining how they expect patches, and if in doubt, ask.  A good understanding of whichever revision control system the project uses is essential.

For example, if the project uses git and has a mailing list then maybe they prefer patches to be submitted via git format-patch and git send-email.  Or if it’s hosted on github then they probably prefer to receive pull requests.  I also highly recommend reading the chapter in the Pro Git book entitled “Distributed Git – Contributing to a Project“.

4. Ensure the patch series is clean and easily reviewable

The most important rules are the following:

• Provide a clear commit message.  If you provide a patch with no explanatory text, it will probably be rejected immediately.
• Ensure there is only one “logical change” per patch.
• Ensure each patch in the series is internally consistent.  It is not OK for one patch to contain a mistake which is then rectified by a later patch in the same series, since this knowingly breaks git bisect.
• Avoid lines longer than 80 characters, because this can make it difficult to review patches.

If you use git, you should ensure you are very familiar with its ability to rewrite history, and regularly use this during development to adhere to the above rules. For example, if you notice a mistake in one of the commits in your pull request, squash the fix into the original commit, and then re-upload the revised pull request (on github this is achieved easily via git push -f).

Fortunately the OpenStack project has written an excellent wiki page covering this topic in detail.  Go read it!

5. Include appropriate test suite and documentation changes within each patch

Tests and documentation are first class citizens within the project, and should be treated with as much respect as the code!

So if the project has a test suite, any change you make must have corresponding amendments to the test suite.  If your patch fixes a bug, it should also add a regression test.  If it adds a feature, it should also appropriate unit/functional tests.  Ideally, modify/write the tests before you change the code, because Test-Driven and Behaviour-Driven Development have many additional advantages.

Likewise, any documentation relating to the code you are modifying should also be changed to remain consistent with the code.

6. Assume the maintainer(s) / reviewer(s) are busy and short of spare time

This is almost always true!  The above principles all help reduce the workload for project maintainers.  Failure to adhere to any single one of them will simply waste their time and will get your patch rejected, probably without even being fully read.  On the other hand, if you avoid these pitfalls, your patches have a good chance of being accepted with gratitude (and even surprise and delight in some cases …)

7. Accept constructive criticism gracefully and react accordingly

If your patches are rejected, resist the urge to get annoyed!  This can be tricky, but it helps if you instead treat the rejection as free training – it’s an opportunity to learn how to improve your coding and collaboration skills.  Carefully consider the reasons given for rejection (if insufficient reasons were given, politely ask for details), and if resubmission still makes sense, modify your patches to take the feedback into account, then resubmit.

Related reading

• https://www.igvita.com/2011/12/19/dont-push-your-pull-requests/

Hope this helps.  Happy hacking!