Rethinking Pull-Requests & Code Reviews in Modern Development Workflows
The Summary & Conclusion
Preface - Who should read this?
Is it worth questioning Pull-Requests and Code-Reviews? This article tackles whether we should weave these into our dev cycles and company culture. It’s a blend of my past writings and feedback from seasoned tech pros like Senior Developers and CTOs. We’ll explore varied viewpoints, weighing the good and the not-so-good.
While I lean towards Continuous Delivery, I aim to give a balanced view. So, whether you code or lead tech teams, this read will offer clarity on Pull-Requests, Code-Reviews, and Trunk-based development. Dive in!
Index of this article
Part 1: Brief Overview of the Current State
Part 2: The Problem with a Code Review Backlog - Why should we rethink branching?
Part 3: Addressing Audience Feedback
Part 4: Industry Perspectives on Modern Development Strategies + Conclusion
Part 1: Brief Overview of the Current State
Code reviews and pull requests (PRs) have become standard practices in the software development lifecycle. They ensure high-quality code, encourage collaboration, and prevent bugs. However, it's becoming evident that these practices can also introduce challenges like stagnant review backlogs, inefficiencies, and compromises on code quality.
So we are talking about a method intended to improve quality, yet I received feedback that the result can be quite the opposite. In this article, I want to highlight that readers and listeners of my audience have experienced different software development methods. So, let's get into it!
What are Pull-Requests and Code-Reviews?
The "Pull Requests" concept is closely linked to GitHub, which introduced the feature to support code collaboration. While it's hard to attribute the invention to a single person, GitHub co-founders Tom Preston-Werner, Chris Wanstrath, and PJ Hyett played critical roles in popularizing it. The feature is now widely used in other version control and hosting platforms and is sometimes called Merge-Requests, like on Atlassian's BitBucket.
A Pull Request is generally a way to review code changes before merging them into a specific branch. This practice serves as a quality check and a platform for collaboration and knowledge sharing among team members. It has become integral to modern software development workflows, enabling developers to discuss changes, identify bugs, and refine code.
Polls: Do You Use Pull Requests and Code Reviews?
I polled developers about their use of PRs and code reviews. Here are the results:
PR + Code Review (Happily): 70%
PR + Code Review (Unhappily): 17%
Trunk Based Development: 9%
Note regarding correctness: James Galyen hinted that this poll could have been misleading since PR+Code Review and TBD together are possible. That’s true. During the next week, I will ask these questions in the context of TBD again and update this section accordingly.
The poll also revealed that several participants who didn't vote in the survey were inclined towards trunk-based development (TBD)1, adding further weight to its emerging popularity.
Finally, in Tech Leader Slackgroups or WhatsApp Groups, the tendency was subjectively clear on the PR+Code Review Side.
Based on my current knowledge base, most developers are using PR + Code Reviews in one way or another. Yet, a significant portion are using trunk-based development.
Polls: Trunk Based Development & Pull Requests
Pull Requests and Code Reviews aren’t required for Trunk Based Development since TBD is based on making changes directly to the trunk or with short-lived branches. The latter approach requires discipline but mitigates problems with very decentralized non-core groups of developers.
In trunk-based development (TBD), would you prefer direct commitments to the trunk or short-lived branches that merge quickly?
Direct changes to trunk 37%
Branch merged after PR+Review 44%
Branch without PR+Review 14%
Other (comments) 4%
Discussion between Senior Developers
Based on the poll I conducted about trunk-based development (TBD), I received various insightful comments from tech professionals that enriched the discussion. Ángel del Blanco Aguado2 stood out for endorsing pair programming, which he feels can be effectively combined with direct commits to the trunk or short-lived branches, depending on the situation.
Paul Hammond3 made a compelling case for using very small branches, arguing that this approach strikes a balance and provides psychological safety for developers.
Richard Smith 4 pointed out that while direct commits work well for smaller teams, transitioning to a more structured approach involving PRs and reviews becomes necessary as a team grows.
James Galyen 5 emphasized flexibility in development workflows, advocating that the chosen method should ultimately serve the interests of customers, developers, and the business.
Additional Views on PRs and Code Reviews for Learning and Collaboration
Additional comments from the poll further expanded the scope of the conversation. One senior back-end software engineer expressed that PR+Review mechanisms can reinforce collaboration when changes are made to the master branch.
A consultant software developer echoed this sentiment, stating that pull requests and reviews are excellent code quality and learning tools. These viewpoints reiterate the importance of considering various development practices, like pull requests and code reviews, as valuable components in a balanced Trunk Development strategy.
Going Straight: Going straight to Trunk and avoiding making PRs
Next to the voices for Pull-Requests, there were clear voices6 against Pull-Requests during TBD and Pull-Requests in general, which refers back to the original topic of this article. Instead of reviewing via PRs, which are irrelevant in the sense of TBD, Andrea L. 7 made an exciting contribution.
In Continuous Integration, it's crucial to understand that there are viable alternatives to pull requests for facilitating code review. Among these, Pair Programming stands out for offering real-time, continuous feedback during the coding process itself. Then there are Periodic Reviews, a structured approach where the team allocates specific times to review all recent changes. Last, Pipeline Approvals insert a human review stage post-integration and automated tests, allowing us to scrutinize code that has already met technical criteria. These methods maintain and potentially enhance code quality, all while keeping the software delivery process agile and efficient.
Source: “Why your team doesn't need to use pull requests“8
A misunderstanding when it comes to Continuous Delivery or TBD
Continuous Delivery (CD) and Trunk-based Development (TBD) are often misunderstood. CD is about always being ready to release, not the frequency of releases. TBD focuses on high-frequency integration, not the elimination of branches or Pull-Requests (PRs).
The ultimate goal for both is to deliver high-quality software efficiently. Rather than debating the use of PRs or branches, the focus should be on creating an environment that supports continuous integration, making it possible to release valuable software early and often.