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.
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.*
- [ ] 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
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.
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.
I like to write blog posts that share exciting new tools I’ve discovered, how-to articles and also the occasional opinion piece.
I live in Prague in the Czech Republic with my family.
Check out the original blog post on my blog.