How to get your open source changes merged
Putting time and effort into a pull request only to have it sit there unmerged for weeks (or months) can be very disappointing and frustrating. Unless you have psychic powers or blackmail material, you can’t force maintainers to act, but there are things you can do (or not do) to help get your changes merged sooner. Here are some tips I’ve picked up, both as a contributor, and now as a maintainer of an open source project.
NOTE: Obviously, projects and maintainers vary. This is my own opinions, based on my own experience with GitHub pull requests, most of which is connected to the SerenityOS project. Your experience elsewhere may be different, but much of this advice will apply anywhere.
First of all, a couple of basics:
Make sure your PR fits the project
Before you even start, make sure that what you want to do is in line with the project’s direction. I’ve seen multiple cases of someone working hard on a feature, submitting a PR, and then having it closed quickly because it does not fit the goals of the project. This is heartbreaking, and can be avoided by talking to the project’s developers first to find out if people want your feature. Projects usually have some kind of chat server or mailing list you can use for this.
Read the contribution guidelines, and follow the coding style
Most projects have a CONTRIBUTING
file, and often a coding style guide, that describe how new code should be structured and written to fit in well with the project. These are probably not an entertaining read, but make sure to read them anyway. Ignoring these is a great way of putting your maintainer in a bad mood, as they have to repeatedly point out small style issues that you could have known about yourself. It gives a strong impression that you don’t care, and is exhausting to review. It also means you’ll have a lot of changes to make. It is better all round to take the time to read them, and write your code in a way that will be accepted.
If this is your first PR to the project, start with something small
There is usually no minimum size for a PR. Small corrections and bug-fixes are a great way of getting used to the review process, how to work with git
or other source control to correct mistakes, and to introduce yourself to the project. Any mistakes you make will be much easier to spot and deal with in a 3-line change than a 300-line one.
Maintainers are human
Maintainers and other reviewers are human beings with other things to do. Most of the time, they are volunteers. That means that when someone sits down to look at your code, they’re using their free time to do so. Value that time! Make it as easy for them as possible to understand and review your changes. By doing so, you will earn their trust and they will prioritize your PRs in the future.
Test your code before you submit it
The project may have some kind of automated testing that runs on PRs. A maintainer might test out your changes for themselves. But you can’t really rely on either of those. It’s up to you to make sure that what you submit is good code. It’s embarrassing to get a review back that your code has serious bugs, or would not even build. Perhaps even worse is if it gets merged, and then significant problems are found and it needs to be reverted! So make sure you test your changes. Your PR does not have to be perfect, but you should be confident that it works and does not break things, and that confidence should be based on trying it out rather than wishful thinking.
Attract attention to your PR
So, your have a PR ready. The first step to getting it merged is getting people to look at it in the first place. In busy projects, there may be dozens of PRs submitted each day, which makes it easy to overlook some. How can you get noticed?
First, give it a good title that fits any guidelines, and makes it clear what your change is. On larger projects, each maintainer will have areas of the code that they are more familiar with, and more comfortable reviewing. Making it clear what you changed will help them decide if it’s something they can, and want to, review.
Give your PR a description. Even if all your commits have clear descriptions of their changes, spend a few minutes writing a summary. Why is your change worthwhile? What does someone need to know about it? Even just copying out the commit messages and formatting them is much better than ‘No description’. Show the maintainer that you care about your work.
Demonstrate what is good about your changes. Is your change visual? Include some screenshots or videos! Did you boost performance? Show off your results with a table comparing the numbers before and after. Motivate anyone who sees it to put in the effort to review and merge it.
The project may have an official way to advertise PRs. For example, the SerenityOS Discord has a #code-review
channel. Don’t spam, but make sure to tell people about your PR, especially if it seems to not be getting attention. And think of ways you could improve how you present it so it does get attention.
Remember that nobody is obligated to review your work. Your work is a gift to the project, but a maintainer’s time is their gift to you. Don’t take it for granted, and try not to become impatient or disheartened if nothing happens for a while.
Keep it manageable
You managed to get a maintainer’s attention and they’re about to start reviewing your changes. How do you make this step go as smoothly as possible?
Firstly, try and keep your PR a manageable size, and on a single topic. There’s no hard rule about this, but the smaller and simpler a PR is, the easier it is to review. And also, the less intimidating it is! Opening up an exciting-sounding PR to see 10,000 lines changed across 50 files is daunting. Ideally you review a whole PR in one session so everything stays in your mind. That means the larger the set of changes, the more time you have to set aside to look through them. People are busy, and so the less time they need to set aside, the better.
Similarly, keep commits simple and understandable. Some projects (like Serenity) require that commits are clear and atomic, but even if not, it is still helpful for getting merged. If each commit is a single change, it is much easier to understand. Especially confusing are commits that move a large chunk of code, while also modifying it. One commit that moves the code, and then another that modifies it, (while still keeping things compiling at each point,) is much simpler to comprehend and faster to review, leading to happier maintainers.
Explain your decisions in the commit messages. Answer questions that someone might have - why you chose one option over another, or why you are adding some code that is not used yet. This saves the maintainer having to ask you later.
Side-note: While comments can go out of date, commit messages never do - they are always connected with a specific change at a specific point in time. They also don’t get in the way of the code. Future developers will thank you when they can look up the
git blame
for a confusing piece of code and read your explanations, instead of puzzling through it for hours.
Once you get feedback
You got a review, and they left some feedback. 🎉 Hooray! 🎉 Now what?
An important thing to remember is that criticisms of your code are not criticisms of you! So try not to take them personally. Everyone makes mistakes, everyone misses things, and everyone has parts of a language or library that they are less familiar with. (Including the maintainers!) Review comments are a sign that they want to help you improve, not an attack on you or your programming abilities.
Online, it’s harder to read someone’s tone, and this is especially true when working with people from different countries. Some people or cultures are more blunt and direct, so try not to see this as rudeness if you are not used to that.
You won’t agree with all the feedback comments, but resist the temptation to argue. Keep in mind that while you may know your code and your feature better than they do, they know the project - otherwise they would not be a maintainer! If their opinion makes sense, try to go with it. They will not always be right, but try to be reasonable. Explain yourself clearly, maybe produce some evidence on the benefits of your approach, but ultimately it is their decision whether your code is right or not. Maintainers are on your side - they want to see your bug fixes and cool new features included, and they want your code to be of good quality. Getting a reputation as stubborn or argumentative will hurt you, as maintainers will be less willing to review code written by someone who refuses to change it.
When making your amendments, remember the project guidelines. Serenity for example requires you to modify your existing commits and then force-push, to keep the commit history clean. This video is very helpful if you are not familiar with how to do this.
Once you’ve pushed your fixes, let the maintainers know. GitHub for example lets you “resolve” feedback comments people have left, to show that you have fixed them. Doing this makes it clear that you are ready for another review. It also has a button to request a re-review, so make use of this, and maybe advertise your PR again using the official channel.
While waiting to be re-reviewed, keep an eye out for merge conflicts, and CI problems that you can solve yourself. Sometimes you think you are being ignored, but actually these is a conflict or issue which prevents people from merging it! This can create needless delays to getting merged, and sometimes cause new merge conflicts over time.
Conclusion
I hope you found this helpful. To sum up, my basic advice is:
- Follow any rules or guidelines the project has.
- Present your PR in a clear, interesting way.
- Tell people about it, without hassling them.
- Keep your PR small and focused.
- Stay positive and listen to feedback.
- Have fun!
~ Sam