Whether you are shipping code from mesmerizing scenic valleys, quaint villages, or from your porcelain throne in a 2BHK flat on floor 17 of a tower in a bustling city …effective code review is a great way to boost developer productivity and reduce technical debt. In fact, for efficient code reviews, a report recommends raising a pull request for 200-400 lines of code (LOC). The report claims this to be an effective way to catch 70-90% of bugs in 60-90 minutes of code review sessions. Anything beyond that significantly dives south in terms of the development team’s productivity. That’s just one of the many pull requests best practices that you’ll find in this insight.
Pull request Best Practices
Here are a few pull request best practices for your development team:
1. Small number of Lines of code (LOC)
As shared above, one of the pull request best practices is to only push a maximum of 200-400 LOC. This could also be just one function or a couple of functions or classes. Ideally, first, your peers will review the recently pushed code, and later a senior developer will do the basic hygiene check.
- Usually, the larger the size of your pull request, the longer a reviewer needs to invest time in reviewing code and giving feedback. One must try to avoid this.
- Small-size pull requests ensure that your peers and seniors do give due attention to your code and that you abide by the principle of agile software development, which recommends continuous value delivery to the end users i.e., incremental changes to the working software
2. Simplify the commit message for the reviewer
In general, a developer first forks the main repository, creates a local clone (git clone), makes changes to the cloned repository, pushes the same to the forked repository (git commit), and then raises a pull request to merge the code in the main repository (git push).
- When committing the code developer adds a message title, and description. A pull request best practice for writing commit messages is to prefer brevity over verbosity.
- Firstly, the title should be treated like an email subject line. Keep it capped at 50 characters or less. The description could be a little elaborate but keep that too within 72 characters.
- Secondly, adhere to writing sentences in imperative i.e., ‘add button’, and not ‘adds button’ or ‘added button’. This convention rules out the confusion for reviewers as declarative sentences often mean the work is already done. Also, when multiple follow-up commits are involved, imperative statements could be confusing. In fact, this helps in quick code rollbacks, if needed.
- Lastly, don’t panic about revisiting school, no need to open the Oxford grammar book, just avoid making declarations and you’ll be fine.
3. Don’t skip the timely review, and don’t overdo it
You might be knowing a couple of people, who take a gym membership, and then forget to hit the gym. It’s common. And so is not reviewing the pushed code. A lot of people, surprisingly experienced developers too, are guilty of this. At times, the coders committing the code themselves approve the code to be merged into the main branch. This could be tragic. Avoid it at any cost. Highly productive development teams comply with the following code review, aka pull request best practices —
- Ensure that the code is reviewed ASAP. Ideally, within a couple of hours. The quicker the developer gets feedback from the reviewer, the better. As the issue/feature remains fresh in the developer’s headspace, s/he can quickly work on the feedback received.
- Also, sometimes developers tend to engage in unnecessary discussions over the best way to do a certain feature or fix a bug in the code comments space. This sounds exciting at the moment but often annoys others who need to collaborate on the project and are forced to read the comments.
- As a precaution, if you’re a reviewer and your plate is full, do not accept review requests.
4. Squeeze value out of engineering analytics tools
Use engineering analytics tools (say ‘Hi’ to Hatica) to measure metrics like cycle time, reaction time, time to merge, code churn percentage, etcetera. Using relevant engineering KPIs & metrics is a pull request best practice for benchmarking if code PRs are done right—
Engineering analytics can help you fix broken PR processes that hamper developers' productivity and become a bottleneck for a team. i.e., pull requests being merged without review, PRs without testing, overlooked or pending PRs, open PRs, reviewers too long (time) to comment, reviewers' unavailability, and code incompatibility with the existing codebase.
For example, a high churn percentage would mean either the requirements/functionality is being changed too frequently, or the code review process is flawed. Similarly, if cycle time is too high for a backlog item, aka software feature, then there could be a multitude of issues including high PR wait times.
Hatica’s dashboards help you get visibility into all these key metrics to have a clear understanding of how your dev team has been doing across the software development life cycle.
Miscellaneous Pull Request Best Practices
- Make use of labels, milestones, assignees, and other relevant query parameters to create informative pull requests. These help in PR segmentation, which might not be very helpful at the beginning of the project. But as the project scales and 100s of developers are working on it, these parameters help them in effective collaboration and improve developer productivity. For example, labels could be a ‘bug fix’, ‘critical bug’, ‘button gradient features’, etcetera.
- If you’re using GitHub, make use of the draft pull request feature to gather early feedback on your code commits, and later, change the state to ready for review for proper code review.
- Avoid force-pushing commits to a PR as this can change your repository history and potentially corrupt the PR. In a shared repository model, if the collaborators base their work on the version before your force push, this may create further conflicts in the future, and loads of re-work. So, keep git force --push as forbidden.
- Try automating the PR process as much as possible. Make use of tools like hub CLI for speeding up the pull request process. Code quality tools can help you automate the due diligence for coding standards and conventions, and test automation tools can assist in bug discovery, and identifying security vulnerabilities.
- PRs must include all the essential unit tests. If possible, arrange for visual aid as this helps the reviewer to save time. For example, if you’re modifying an existing feature — include ‘after’ and ‘before’ screenshots.
For additional tips on optimizing your pull requests, see our guide for tips on How to write better Pull Requests.
Pull requests Vs Merge Requests Vs Code Reviews
- Pull request (PR) is the process of adding new code and/or bug fixes, aka patch code to the main repository which is implemented using a version control system like Git.
- Merge request is part of pull request process and refers to the process of integrating modified/new code to existing repository.
- Code review too is part of the pull request process, where senior developers review the submitted source code i.e., functionality implementation, code compatibility, conventions, etcetera. It’s normal for first-time coders to freak out when they are shot with phrases like code reviews, merge requests, pull requests (PR), main repository, branch repository, commits, code patches, and whatnot.
In fact, thanks to the explosion of new CI/CD tools like Bitbucket, Gitlab, Azure pipelines; a plethora of software development paradigms; and the globalization of dev teams (hey, distributed remote teams), even experienced developers sometimes get confused and feel dizzy upon hearing some of these phrases.
Why else do you think Sr. developers are prone to receding hairlines?
Okay, don’t get hysterical, that was meant to be a joke.
But on a serious note, developers often use different words/phrases interchangeably to refer to the same process.
For instance, Ryan who uses Phabricator may refer to code reviews as diffs. But Ryan’s new colleague, who has just joined his organization refers to the same as pull requests. While some developers also call it ‘merge requests’ but what they mean is ‘pull requests’.
Just so that we’re all on the same page — before we discuss pull request best practices, let’s clarify what we mean by pull requests. In Git world, it basically means —
git fetch and git merge
Summing it Up!
Abiding by pull request best practices improves code quality and readability for collaborating developers, promotes consistency, and keeps a code history for switching back or fixing bugs efficiently. Following PR best practices is all about speeding up your project’s development and enhancing developer productivity. You may consider engineering analytics tools to further augment the developer outcomes.
Subscribe to the Hatica blog today to read more about unblocking developers, and boosting productivity with engineering analytics.
