Great things are usually built by teams, not by lone wolves. The myth about the 10x developer sitting alone in the basement is often exaggerated. In practice we’re usually working in a team, building (hopefully) great things together. Aside from the code we write, how we collaborate with others, our work ethic and ways of working have big impact on the success of a project.
The code review practice is widely adopted for increasing overall code quality and decreasing number of bugs that reach the QA team / end user. On top of these, I believe reviewing and discussing code written by your peers is the best way to share knowledge within the team, to onboard new members faster and to quickly adopt new patterns as a group.
Below are my top tips for being a great team-player, making your pull requests easier to read and understand so they can get approved as fast as possible.
TL;DR — The best advise for creating great pull-requests that are easy to understand and a pleasure to approve is to show that you care – for the code, for your fellow team members and the process around it. Throughout the whole journey think about the people that’ll read your code and put yourself in their shoes. Always try to make it as easy for them as possible.
1. Isolated
This is probably the number one rule for opening great PRs. Follow the Single Responsibility principle – do as little as possible in a pull request and modify only one logical part of the code. We often fall in a rabbit hole trying to fix the world (codebase in our case) at once. Instead make one small isolated change per pull request. If you need to do a lot of refactoring to implement a feature, split your work in incremental steps. The riskiest thing you can do is to open one huge PR that changes half of your codebase. Better approach is to modify only one logical layer of your application in a single PR. So if you change the networking layer, the database, few use-cases and a bit of UI in a PR – you have lots of room for improvements!
2. Small
This follows rule #1, but adds another dimension to it – size! When it comes to pull requests, size DOES matter – the smaller the PR, the better! Even when making an isolated logical change, sometimes the sheer number of files makes it harder to read and really grasp a PR. When possible, try to split the work in smaller chunks.
3. Split business vs style vs refactoring changes
Always split the business logic / feature implementation / bug fixing from reformatting, rewriting or code re-styling. A great example is fixing a bug in an old Java file. The project now uses Kotlin, so that’s a great opportunity to transform the code to Kotlin, right? It always depends, but generally I don’t think so! This will make the bug fix harder to spot in the newly generated Kotlin code. A better approach would be to open a tiny PR that fixes the bug in the old Java code, immediately followed by another one for the Kotlin conversion.
Same logic applies to major code refactoring. I’d always strive to make this separately from implementing a new feature or fixing a bug. This helps fellow team members focus on only one thing and really understand your rationale behind it – be it the refactoring, be it the bug fix. As a bonus – following this rule goes hand in hand with the previous two.
4. Meaningful branch name
Always try to add a meaningful name to the branch you’re working on. At the very minimum, create the branch in a folder related to the change type (bug fix / feature / tech improvement / release / etc). Always include the ticket number from your work tracking system (JIRA, Trello, BugZilla, etc) in the branch name. This gives an immediate pointer to the readers of your PR where they can get more information about the change. simplez_change doesn’t mean much to a reader, whereas bugfix/andr-435 or bugfix/andr-435-details-page-font-size give lots more.
5. Include the ticket number in each commit
By this point you should have the ticket number in your branch name. This is helping your reviewers find more information. Assuming your branches are short lived (and they should be if you followed all rules so far!), the branch name won’t be of much help in the long run. Putting a reference to the ticket number in each commit will be extremely helpful in the long run though!
The biggest benefit of keeping the ticket number in each commit is that you’ll have extra information, context and reasoning for each line of your code. Annotating it (using git blame or git annotate) will show you which commit last changed this line of code, which in tern has a reference to a work item in your system. Each change is traceable and can be easily attributed to a specific feature. This has saved my team countless times! It comes especially handy when you’re working on a particularly nasty / dodgy / hacky piece of code and the extra help is most needed.
It takes a bit of practice to get into the habit of repeating the ticket number in each commit message but believe me – it’s totally worth it! You can enforce this with a simple git hook. If there’s a team member that’s not fully onboard with this – be sure to call this out at pull requests. He/she will definitely change their mind the first time this little tip helps them solve a nasty bug!
6. Great title and description
That’s your chance to give extra explanation what you’ve done and why. This should be proportionate to the complexity of the PR, not its size. No need to write a 3 page essay to explain a changed font. Alternatively, a very subtle change to fix the race condition that causes the biggest crash on production needs a lot more context.
Keep the title concise and the description nicely formatted. Explain the “Why“ and the alternatives you’ve considered. Leave the “How” for last … and include it only if the code isn’t self-explanatory (hopefully it is).
7. Include visuals where applicable
A picture is worth a thousand words! If your PR includes visual changes, best way to showcase what you’ve done is to include a screenshot / gif / video / before-and-after comparison. Visuals make your PR more interesting and people tend to review these faster.
8. Include additional info where applicable
The more information available to the reviewers, the faster they’ll do their job. Where applicable, include extra links that might make their life easier – link to the design requirements, the API specs, repository with dummy data, related work items, link to a similar bug from a month ago, etc.
9. Use PR templates
Most hosted git solutions support the creation of PR templates of some sort. This includes GihHub, BitBucket, GitLab etc. This gives a bit of extra structure to your team and helps building the good habits. Templates are most helpful to newly formed teams. Once a team is “gelled”, templates shouldn’t be needed, as all members should have “a feel” of what’s right.
Hope these rules help you create great PRs that are accepted faster. Please let me know in the comments if I’ve missed a good practice that helps your team. Thanks!
Happy coding!