How to send good pull requests on GitHub

Make sure the PR is needed

This is especially important if you're contributing to a repository you haven't worked with much before. Do some research in the existing issues and PRs in the repository - including closed ones. Is this change already being discussed somewhere? Was it proposed before and rejected? The code you want to change - is it there for a good reason?

GitHub offers reasonably good search capabilities in case the project has a large log of issues in PRs. It's not perfect, but by running a few searches with probable keywords there's a good chance to find something. Another thing I often do is search the commit history of a project for relevant information (git log --grep).

Demonstrating some due diligence goes a long way in showing the repository owner that you're a serious contributor.

Have an open issue linked in (optional)

An important tool of modern software development discipline is having an open issue (or bug, or ticket, or however else it's called in other systems) to discuss some problem or some missing feature we want to address.

An issue is more general than a PR description. An issue describes a problem; a PR describes a solution to that problem. Some issues require multiple PRs to be solved, and interlinking all these PRs through the issue is critical for later attempts at archaeology.

If in doubt - open an issue. Add all the context there. The PR will then reference the issue with a #<issue number> tag - this is something GitHub understands and will add a link between the two. A PR can also say Fixes #<issue number> if merging this PR means the issue is fully solved.

One of the most frustrating experiences for a repository maintainer is getting a PR without sufficient context of what it attempts to solve and why. Having an open issue with all the details is the best way to establish this context; the next sections address some additional ways.

I marked this section as (optional) because an issue isn't necessary in some cases. For example, typos in comments typically don't require an issue and a PR carries sufficient context. Minor changes in documentation also don't require issues in most cases.

If in doubt, create an issue. Linking this to the previous section - if an issue describing the problem already exists, make sure to link your PR to it - maintainers love PRs that solve open issues.

Write a useful PR title

This advice will read a bit like "what makes a good git commit message".

The PR title is extremely important. It's what people see when listing all open PRs. It's also commonly translated to be the first line of the merged commit, and shows up prominently in git log, etc. Take special care in crafting the PR title to be descriptive and useful, but not too long.

Some large repositories have special guidelines for writing PRs. For example, the PR title would start with the component name - "storage/remote: increase widget timeout". Look around - how do other PRs (that were successfully merged) look? Is there any contribution guide in the repository that details these conventions?

Write a detailed PR description

This ties strongly to the "Have an open issue linked in" advice. If the PR requires a long background description, it's better to do this in an issue and have a link in the PR. If there is no issue for some reason, the burden is on the PR description to explain the motivation for the change, and the approach taken in it.

But PRs and issues are also for diferent purposes. Sometimes, a PR description will have information that doesn't belong in an issue, such as details of the specific approach taken in the PR, benchmark numbers for this PR, etc.

The PR description will make it into the git commit log - add as much detail as you can. The repository mainainer can later tweak the commit log so they will remove things they don't need; if in doubt, add more details.

Adhere to the project's coding standards

Does the project have a contributors guide? Spend a couple of minutes looking for it.

Look at other PRs that were merged - what did their authors do?

Look at some of the existing code in the repository - try to match the style of your PR to the prevailing style in the existing code. Doing this shows the maintainer that you're a serious contributor who cares about the long term health of the project.

Be attentive to the smallest details: how much whitespace does the code have, including in comments? Is there a specific writing style - Oxford commas, one or two spaces after a period, and so on?

Add tests

If you're adding new code - make sure it's tested. Either add new tests, or point out in the PR description which existing tests cover it. If a test is too hard to add for some reason, explain why.

For changing existing code the situation is a bit more nuanced. Is the change addressing a current test failure? Which one? Which tests are affected by the change? Should new tests be added?

Spend a few minutes thinking about this and documenting your conclusions in the PR description. Again, this shows the maintainer that you're a serious contributor and your PR is more likely to get attention.

Make sure all tests pass

Many GitHub repositories have integration with CI tools, whereupon each PR gets automatically tested and the CI system adds notes to the PR about its passing or failing.

After sending a PR, watch out for this and address any failures. Maintainers are unlikely to pay attention to PRs that break the build or tests.

If the repository has no such tool, make sure to run all the tests you find in the project and ensure that your change doesn't affect them negatively. If some test breaks because it's bad, make sure to fix it. If some tests fail with or without your change, make sure to call this out in the PR description.

Be patient and friendly during code review

Once the PR is finally out, the contributor's task isn't done yet. In fact, most of the work may yet be ahead. The code review process is an important tenet of modern software development, and knowing how to behave is critical for success.

Whole book chapters have been written on code reviews, so I'll keep it short and simple here, focusing on open-source contributions. When sending a PR for a work project, it's obvious we should be extra friendly and kind with colleagues, right? Right?

But what about open source? You found an issue in some OSS project you use, and are sending a PR. Good for you! Do you expect the maintainer will be delighted about the contribution? Well, not necessarily, and it really depends on your demeanor.

OSS maintainers are notoriously overworked and underpaid. Sometimes they just want stability - as few changes as possible. Clearly if you report a critical bug they will likely be happy to fix it; but 99% of PRs are not for critical bugs - they are for minor bugs and new features. Here the PR presents a dilemma for the maintainer - someone is sending code, and this someone is very likely going to completely disappear after the PR lands, so the responsibility for the code moves to the maintainer. It's not surprising that in many cases, maintainers are cautions and even suspicious of every change.

When answering review comments be patient and friendly. Assume good intentions - the maintainer is taking extra burden to maintain additional code (especially with feature PRs) and it's their right to scrutinize it and take their time. Multiple rounds of reviews may be required. Be sure to be responsive and attentive to detail - acknowledge all comments, either by doing what the reviewer suggests, or (kindly) explaining your point of view. Add more tests if they ask you to (and you can't point to existing tests covering the same thing), add more comments if they ask you to.