Great Pull Requests
Pull Requests are now the standard for developing software in a team or open source setting. Contributors request a set of code changes to be approved for inclusion by one or more members of the maintainer team in order to uphold the healthy evolution of a codebase.
However, composing the right kind of Pull Requests is essential for your team to be productive and efficient.
Let's boldly split into two groups the activities on which most of the Software Engineers spend their workday as individual contributors at the average Tech Company or Startup:
- Group a.
- Group b.
- Mentoring or helping a teammate.
- Replying to email & instant messages.
- Fixing a broken pipeline or incident.
- Keeping up with trends of the field.
- Reviewing Pull Requests.
It may not seem evident, but your team could spend significantly less time on the second group of activities by simply producing better Pull Requests.
What is a Great Pull Request?
You know it when you see it; a good looking set of changes, nicely wrapped, with all the information you need to do your thing. The incoming change might not be simple, yet it is simple to review: clear, beautiful, and it makes you warm and fuzzy inside.
Simpler and quicker to review
On a Great Pull Request, reviewers don't waste time figuring out the context, rationale, and implications of an incoming change or feature. While context-setting might not be a big issue on a small team, it makes all the difference on medium to large teams, where it becomes impossible keep track of the details of what everybody else is currently working on.
Share and push knowledge growth
A Great Pull Request is better than the average blog post when introducing design patterns, technologies, techniques or best practices. The reviewers get a practical and contextual case study for associating abstract concepts or techniques. Not only is the reviewer pushed for growth, but the author becomes encouraged to research further in order to support and justify the strategy and consequences of their proposed changes.
They boost or maintain the quality of a codebase. Thoughtful changes become contagious amongst the contributors; they set the bar for reviewers and contributors. Good codebase quality directly correlates to fewer incidents and outages, which translates productive and happier teams.
Introduce documentation and a runbook for debugging
They help new team members commence and understand new codebases; they simplify and expedite getting production issues under control; since they become snapshots of the entire story in a single place, no time gets wasted tracing and investigating root causes, how things work, or stabilization strategies.
Dangers of Bad Pull Requests
Bad Pull Requests are less likely to be approved on a single review round; as reviewers request clarifications, they increase the chances of falling into never-ending change requests. Even worse, they end up getting closed, wasting everyone's time.
Slow down product development
Bad Pull Requests are procrastinated upon; they become "hot potatoes" team members delay reviewing until someone has to "take one for the team."
Bad for the author's career
Bad Pull Requests damage the reputation of their authors; they diminish the team's confidence in their qualifications and raise concerns amongst the management, making it harder for authors to progress in their career.
Do's and Don'ts
Do: Thoroughly review your code before asking someone
It is unlikely anyone will grasp the content and extent of proposed changes better than the authors themselves; "rough edges" and "human errors" should be polished before requesting others to spend their time on it.
Do: Provide context
A Pull Request that doesn't provide background becomes a "puzzle" for reviewers; Reviewers will make assumptions to uncover the reasons and intent of the author, which can eventually be mistaken and lead to unwanted outcomes. This leads to delays and overhead for the reviewers to either seek additional clarifications from the author or to research on their own. At very minimum, a Pull Request description should answer the questions "What?", "Why?" and "How?"; Writing "bug fix" is never an acceptable description.
Do: Keep it short
A long Pull Request increases reviewing complexity; small atomic changes allow reviewers to wrap their heads around the change quickly rather than dealing with a spaguetthi of changes spread across multiple files. When short Pull Requests are not technically feasible, multiple semantic commits can help get a hold of the components and progression of the development. Given multiple commits can always be "squashed" before merging, there is no need to cram everything into a single commit.
Don't: Address more than one concern at a time
A Pull Request that combines unrelated code changes pollutes the repository's history and unnecessarily complicates rolling back to previous versions during incidents or outages.
Don't: Include the wrong reviewers
A review from somone who is not familiar with the codebase or is not proficient with the language/technology is just overhead without value.
Don't: Lower the test coverage
A Pull Request that doesn't include Unit or Functional tests is a bad Pull Request. There are few cases where including new tests may not be required, but authors should always communicate these reasons.
If you or your team are struggling to get things done, maintaining code quality or merely feeling there are not enough hours in a day; look into your Pull Requests. Great Pull Requests are the one thing any team of any size can start doing today with outsized investment returns in both the short and the long term.