DEV Community

Cover image for How To Open a Great Pull Request
Jeff Edmondson
Jeff Edmondson

Posted on • Updated on • Originally published at jeffedmondson.dev

How To Open a Great Pull Request

Pull Request Meme

🥅 Your Goal

When opening a Pull Request (PR) your goal is to get your code merged into the main branch so that it can be released & a part of the code base. However, before your code can be merged it first must be reviewed. The pull request is where this review will take place. In order to get your code reviewed & approved as fast as possible, you want to create a great Pull Request. This Pull Request should make the code reviewers' life as easy as possible. If the code reviewer has to leave the pull request to get more information, then it is not a good pull request. All relevant information should be included in the Pull Request. There is nothing worse than a huge number of file changes and an empty PR description form. There are a bunch of different Pull Request templates that you can find on the internet that are great, but regardless of which template you end up using, make sure the following information is included.

🧐 What

You want to make sure that you adequately describe the requirements that your change is implementing or fixing. Having the code reviewer have to guess what your change is doing is never a good idea. If you have a ticket number for your change make sure to link it.

🤓 Why

Now that the reviewer knows what your code is trying to accomplish it is time to give some context as to why you are doing things the way that you are doing them. You don't, and shouldn't, explain every single line of code. No reviewer is going to read an entire dissertation about making a page more mobile responsive. However, it is good to explain things from a high level. It is also good to explain why you chose to do things a certain way. Try to think of the possible questions that your reviewers might have before they ask it. This will save time in the long run. If you are referencing any outside material make sure to link it.

✅ Validation

As a developer I have trust issues. I've seen release branches get broken too many times. So to ease your reviewers' doubts make sure to include validation that your code is doing what it is supposed to be doing. Validation can be a variety of different things. If you are working on a frontend change make sure to include screenshots or videos. GitHub now allows you to add videos to your PR. Also ensure that your screenshots or videos take into consideration various screen sizes such as mobile and desktop. Working on creating a new / editing REST endpoint? Add a curl and its response to the PR.

🌴 Keep your GIT History Clean

Save yourself time & energy and always ensure that your git history is up to date with the main branch. Merge conflicts are the worst and best to be avoided all together. However, sometimes they cannot be avoided. In that case you need to be proactive about resolving them instead of having someone tell you to resolve them. This just increases the time that it takes to get your code merged into the main branch.

You also want to make sure that you are writing good & descriptive commit messages. "Fixed the bug" & "Should work now" aren't gonna cut it. This holds especially true if you are not doing a Squash & Merge and instead creating a merge commit for each commit when merging to the main branch.

💬 Communicate

Your code is not you. I am going to repeat that because it is so important: YOUR CODE IS NOT YOU. You should not take offense when reviewers request changes. You should try and take this as a learning moment, be grateful that you are able to learn a new or better way of doing something. Communicate openly with reviewers. You should answer any question that they may have while providing them with links & examples to make their life as easy as possible.

Discussion (2)

Collapse
lukzmu profile image
Łukasz Żmudziński

One of the things that are really important in any project (regarding your Validation part) is to set CI/CD, so that most of the validation is done for you automatically, either through linting or tests.

Squash and Merge is a bit controversial, since you do not keep the history of commits that way, making it much harder to browse blame through individual contributions.