Pull Request Perfection: A Deep Dive Into My Setup Strategy

Written on 2024-01-13 by Adam Drake - 8 min read

I work in a team of developers and like pretty much every development team out there we have Pull Requests for reviewing code being merged into certain branches. Over the years I have adopted useful habits that help my colleagues understand exactly what my PRs are about and enable them to review them as quickly and efficiently as possible.

In this article I will highlight the main steps I take on every PR. This not only improves the efficiency and harmony of the team but also acts as a safety net for me to make sure I am not submitting something that isn’t ready yet.

Use templates

This is always a good idea when working in a team because it keeps the structure of your PR’s description text the same across all your PRs. This will enable each member of your team to quickly distinguish each part of the text to find relevant information.

Developers are busy and often down their own little rabbit holes. If they have to context switch to review your PR you want to make sure you make it as easy as possible for them.

It also allows you to determine exactly what should be in your PR’s descriptions so you can enforce each team member to use specific templates and to provide all required information. This prevents unneeded back and fourth communication between team members to find out certain information.

An example of a PR template in Markdown:

# Pull Request Template

## :memo: Frontend/Backend

## :memo: Description

*A brief but consistent description of the change(s)*

### :briefcase: Type of change

- [ ] New Feature
- [ ] Bug fix
- [ ] Improvement
- [ ] Deployment

### Related ticket(s)

- *The URL of the related ticket*

### Relevant logs and/or screenshots

(Paste any relevant logs - please use code blocks to format console output, logs, and code as it's very hard to read otherwise.)

### Additional comments

*Any additional comments that are relevant for the merging process or the changes themselves.*

## Checklist:

- [ ] My code follows the style guidelines
- [ ] I have performed a self-review of my own code
- [ ] I have properly commented my code
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added evidence of the fix/feature effectiveness in the ticket
- [ ] New and existing unit tests pass locally with my changes
- [ ] I have checked my code and corrected any misspellings

Specify the type of change

This will be part of the PR template hopefully but it’s always a good idea to give the reviewers an idea of the type of change they are looking at. Developers are busy and often down their own little rabbit holes. If they have to context switch to review your PR you want to make sure you make it as easy as possible for them. Therefore make it clear what type of change they are reviewing. This can be in the description or with labels (more on labels later!). The main types of changes will be:

New Feature
Bug Fix
Improvement
Deployment

Write a clear concise description

Writing a clear and concise description gives the reviewers a better idea of what they are reviewing. It’s important to specify relevant and important details about your changes BUT you don’t need to write a novel. If the text is too long people won’t bother reading it. If it’s not clear enough they reviewers may focus on the wrong thing in their review. Bullet points can be very useful in these types of descriptions as it enables you to clearly separate important points and for the reviewer to quickly skim read.

However, they need to be used correctly and updated properly otherwise they quickly become more annoying than useful. So whenever you have a PR open pay particular attention to the labels and make sure all of them are appropriate.

Include screenshots or videos where appropriate

When working on Frontend code the majority of your changes will probably touch some UI. This is all very visual so it really helps reviewers if you include screenshots or videos of the changes implemented.

Sometimes words are simply not enough in describing the changes in a PR. A screenshot with a specific section highlighted can instantly show the reviewer what the change was. This really helps them focus their attention on the critical parts of the code changes.

Specify any tests written

Tests aren’t every developers favourite part of the job but the amount of times a test has saved me requires me to make sure tests are included in almost every PR I submit. I also like to highlight these to the reviewer so they can be checked.

However, it is also easy to fall into the trap of thinking your feature will be only used in one or two particular ways. Reviewers can often bring up scenarios you have never thought of.

Tests provide an opportunity for you to think about the ways in which a certain feature or functionality will be used. It enables you to quickly and efficiently battle test your code. However, it is also easy to fall into the trap of thinking your feature will be only used in one or two particular ways. Reviewers can often bring up scenarios you would have never thought of so making sure reviewers are aware of tests allows them to do this.

Highlight anything specific with inline comments

Occasionally there may be something in the code that you think isn’t entirely obvious. For example whilst working on a file you may have noticed a small bug and you took the opportunity to fix it. Granted you could have raised a Bug ticket but due to the size and ease of fixing this bug you took the initiative and fixed it there and then. However, it has nothing to do with the feature you are working on. Therefore, a quick inline comment can clarify to the user why this piece of code changed.

This whole approach requires an agreement within the team about these type of situations. Some teams will be super strict and everything must be reported. Others will be more relaxed and enable a few things to be done in a PR. Every team is different. The point here though is to have empathy for the reviewer and put yourself in their shoes. If you feel a certain change is not obvious then provide further explanation.

Use Labels Correctly

Labels are very useful in PRs because they enable you to quickly classify the type of PR it is and what changes have been done. Labels like ‘Bug Fix’, ‘Work In Progress’, ‘Project Nature’, ‘Ready for Review’ all give the reviewer an idea about what the PR is about and it’s current state.

Labels need to be used correctly and updated properly otherwise they quickly become more annoying than useful.

This avoids unnecessary questions and saves everyone’s time. However, they need to be used correctly and updated properly otherwise they quickly become more annoying than useful. So whenever you have a PR open pay particular attention to the labels and make sure all of them are appropriate. Your team mates will appreciate it!

Make sure any automated tests are passing

This might seem like an obvious one but the amount of times I have been asked to review a PR and then to find that some of the automated tests haven’t passed would probably surprise you.

Simple tests for things like linting, type checking, unit-tests etc are standard practices in Deployment Pipelines these days and help ensure the quality of your codebase. When submitting a PR make sure these tests are all passing. It’s very annoying to go and review a PR to then find out there is a type error. So do your colleagues a favour and make sure all tests are passing.

Double check all information before submitting for review

This is just good practice for many things in life. Double check your work before you submit it for review. It takes a couple of minutes and I can guarantee that probably around 50% of the time you will find some small error or mistake that you can right before anyone else takes a look. This sounds simple but in practice it can be tricky. You are keen to get your code reviewed and merged, you’re keen to get onto the next task, you think everything is fine… I get it. Just take a breath. Go through your PR slowly and methodically putting yourself in the shoes of a potential reviewer. Make sure that you’re submitting the best work you can.

Use a checklist — This can really help you in times when there are high demands on the team. It’s a simple but effective strategy and requires less cognitive power on your side.

Conclusion

The main message I would want to get through to anyone doing PRs is to have empathy for the reviewers and to make sure you cover all the simple things at least — Labels, valuable descriptions, screenshots, tests are passing… Reviewers are having to context switch to review code, so making that transition as smooth as possible will really aid them in their review. This will lead to reviewers actually looking forward to reviewing your PRs and help the whole team progress more smoothly.

A little trick I sometimes use is to imagine my PR will be reviewed in front of the whole team live. This really focuses your attention and makes sure you are covering all your bases and submitting the best PR you can.

Tweet this article Discuss on Twitter

Written by Adam Drake

Adam Drake is a Frontend React Developer who is very passionate about the quality of the web. He lives with his wife and three children in Prague in the Czech Republic.