The Case Against Pull Requests - 6 minutes read
The Case Against Pull Requests (And How to Fix Them)
Pull requests are an industry-standard but what if the alternative is vastly better?
Ever held an opinion so strongly that you ignored anything that proved it wrong?
I have been guilty of this far more often than I would like to admit. I didn't believe that Utility CSS was anything but an absurd idea until I tried it out. Soon, I realized I had been wasting hours writing needless CSS again and again.
If we questioned these assumptions—which I often call traps—we would realize how much productivity is being lost in doing mundane, repetitive work.
And that brings me to a great example of this: pull requests. The rise of Github made them almost an industry standard of how development should be done. Developers create branches for their changes and file requests to get them merged.
Pretty simple, isn’t it? For a stable open source project that accepts contributions, the answer is yes. It is an ideal workflow for that kind of project. But where pull requests fail developers are projects needing agility. In a fast-paced environment, they impose a heavy tax on developer's efficiency.
Consider an example: you're part of a small team and you've to fix a link in your app, what would a pull request workflow look like?
All the while your branch could fall back behind master, needing you to ensure that you're still in sync. For a tiny fix, the developer had to go through multiple repetitive steps. Imagine doing several fixes like that—to correct spelling, to fix a CSS bug—and creating, checking out, syncing branches is itself a chunk of your work.
You could batch your changes, but it’s always hard to anticipate them (no wonder you have branches like ). Additional problem is that there's an added overhead of creating release branches and merging them back into master.
Of course, you should have someone reviewing your code, but the entire overhead of creating branches for small changes is terrible—and as we would see, avoidable.
The Google Chromium team's workflow is radically different. Their developers don’t create branches at all; everyone commits on the same branch—the master. In a post, Aaron Boodman, a developer on Chrome’s team writes:
On many projects, it’s common to branch the source code to work on major new features. This wouldn’t work in Chrome because we release every day. We can’t tolerate huge chunks of new code suddenly showing up in trunk because it would have a high chance of taking down the canary or dev channels for an extended period. Also, the trunk of Chrome moves so fast that it isn’t practical for developers to be isolated on a branch for very long. By the time they merged, trunk would look so different that integration would be difficult and error-prone.
(btw, this is also called trunk-based development).
The idea might sound appalling but it makes sense. Google Chrome moves at a rapid pace and isolated branches would make it difficult to keep multiple developers in sync. Keeping everything in master makes sure that “all developers are always working with the latest and greatest source.” The releases are picked from the commits in the master. Here's how it might look like:
But what about unfinished features? They are protected by flags/switches. The code is shipped but it isn’t visible to the end-users until the flag is turned on. This practice is not just particular to Chrome’s team, it is practiced at scale at both Facebook and Google company-wide.
Facebook has a tool they call “Gatekeeper” which allows them to be in control of who can see what code live on the service at any given time. As Rossi points out, right now on Facebook.com there is already the code for every major thing Facebook is going to launch in the next six months and beyond! It’s the Gatekeeper which stops us from seeing it. — The Next 6 Months Worth Of Features Are In Facebook's Code Right Now (But We Can't See)
There’s another aspect of it: code reviews. Instead of being branch-based, they are commit-based. When a developer pushes a commit, they are required to get it reviewed by other people who have the same focus area. When it’s approved, it can get merged.
In summary: everyone commits on master, unfinished features are hidden by flags, and code reviews are commit-based.
In contrast to pull requests, this model is superior in multiple ways.
The main distinction is that you see commit as the unit of change, not the branch and that makes all the difference.
The next question is can you adopt the same workflow as Chromium’s team if you’re using Github/Gitlab?
Unfortunately, I couldn’t see how you could have the same style of code reviews as Google’s. Gerrit is an open-source tool but it's almost impossible to use it with conjugation with Gitlab/Github. It has to be installed on a server and the UI isn't pleasant to use.
But it’s possible to build an alternative. There could be a tool to which developer push their changes first where other team members can review them. You can't push your commit to master until your changes are approved.
If there are any suggestions, the developer would need to modify the commit (using amend or interactive rebasing) and push it again. This gives a clear view of what was changed and the reviewer understands if their suggestions were incorporated. When the reviewer approves the commit, it can be pushed to master.
You get benefits of code review without sacrificing your whole setup. Sounds interesting? I am prototyping something like that. You can fill this form if you would like to try it out when it's ready
Source: Dev.to
Powered by NewsAPI.org
Keywords:
Alternate history • Utilitarianism • Cascading Style Sheets • CSS (band) • GitHub • Software developer • Computer file • Debian • Open-source model • Software development • Application software • Distributed version control • Workflow • Needing You... • Cascading Style Sheets • Software bug • Real-time computing • Chromium (web browser) • Workflow • Software developer • Branching (version control) • Commit (version control) • Branching (version control) • Software developer • Graphical user interface • Source code • Time • Google Chrome • Software developer • Software developer • Source code • Command-line interface • Source code • Google Chrome • Facebook • Google • Facebook • Gatekeeper (macOS) • Facebook • Facebook • Gatekeeper (macOS) • Facebook • Workflow • Chromium (web browser) • GitHub • GitLab • Code review • Google • Open-source model • Programming tool • Covalent bond • GitLab • GitHub • Server (computing) • User interface • Code review •
Pull requests are an industry-standard but what if the alternative is vastly better?
Ever held an opinion so strongly that you ignored anything that proved it wrong?
I have been guilty of this far more often than I would like to admit. I didn't believe that Utility CSS was anything but an absurd idea until I tried it out. Soon, I realized I had been wasting hours writing needless CSS again and again.
If we questioned these assumptions—which I often call traps—we would realize how much productivity is being lost in doing mundane, repetitive work.
And that brings me to a great example of this: pull requests. The rise of Github made them almost an industry standard of how development should be done. Developers create branches for their changes and file requests to get them merged.
Pretty simple, isn’t it? For a stable open source project that accepts contributions, the answer is yes. It is an ideal workflow for that kind of project. But where pull requests fail developers are projects needing agility. In a fast-paced environment, they impose a heavy tax on developer's efficiency.
Consider an example: you're part of a small team and you've to fix a link in your app, what would a pull request workflow look like?
All the while your branch could fall back behind master, needing you to ensure that you're still in sync. For a tiny fix, the developer had to go through multiple repetitive steps. Imagine doing several fixes like that—to correct spelling, to fix a CSS bug—and creating, checking out, syncing branches is itself a chunk of your work.
You could batch your changes, but it’s always hard to anticipate them (no wonder you have branches like ). Additional problem is that there's an added overhead of creating release branches and merging them back into master.
Of course, you should have someone reviewing your code, but the entire overhead of creating branches for small changes is terrible—and as we would see, avoidable.
The Google Chromium team's workflow is radically different. Their developers don’t create branches at all; everyone commits on the same branch—the master. In a post, Aaron Boodman, a developer on Chrome’s team writes:
On many projects, it’s common to branch the source code to work on major new features. This wouldn’t work in Chrome because we release every day. We can’t tolerate huge chunks of new code suddenly showing up in trunk because it would have a high chance of taking down the canary or dev channels for an extended period. Also, the trunk of Chrome moves so fast that it isn’t practical for developers to be isolated on a branch for very long. By the time they merged, trunk would look so different that integration would be difficult and error-prone.
(btw, this is also called trunk-based development).
The idea might sound appalling but it makes sense. Google Chrome moves at a rapid pace and isolated branches would make it difficult to keep multiple developers in sync. Keeping everything in master makes sure that “all developers are always working with the latest and greatest source.” The releases are picked from the commits in the master. Here's how it might look like:
But what about unfinished features? They are protected by flags/switches. The code is shipped but it isn’t visible to the end-users until the flag is turned on. This practice is not just particular to Chrome’s team, it is practiced at scale at both Facebook and Google company-wide.
Facebook has a tool they call “Gatekeeper” which allows them to be in control of who can see what code live on the service at any given time. As Rossi points out, right now on Facebook.com there is already the code for every major thing Facebook is going to launch in the next six months and beyond! It’s the Gatekeeper which stops us from seeing it. — The Next 6 Months Worth Of Features Are In Facebook's Code Right Now (But We Can't See)
There’s another aspect of it: code reviews. Instead of being branch-based, they are commit-based. When a developer pushes a commit, they are required to get it reviewed by other people who have the same focus area. When it’s approved, it can get merged.
In summary: everyone commits on master, unfinished features are hidden by flags, and code reviews are commit-based.
In contrast to pull requests, this model is superior in multiple ways.
The main distinction is that you see commit as the unit of change, not the branch and that makes all the difference.
The next question is can you adopt the same workflow as Chromium’s team if you’re using Github/Gitlab?
Unfortunately, I couldn’t see how you could have the same style of code reviews as Google’s. Gerrit is an open-source tool but it's almost impossible to use it with conjugation with Gitlab/Github. It has to be installed on a server and the UI isn't pleasant to use.
But it’s possible to build an alternative. There could be a tool to which developer push their changes first where other team members can review them. You can't push your commit to master until your changes are approved.
If there are any suggestions, the developer would need to modify the commit (using amend or interactive rebasing) and push it again. This gives a clear view of what was changed and the reviewer understands if their suggestions were incorporated. When the reviewer approves the commit, it can be pushed to master.
You get benefits of code review without sacrificing your whole setup. Sounds interesting? I am prototyping something like that. You can fill this form if you would like to try it out when it's ready
Source: Dev.to
Powered by NewsAPI.org
Keywords:
Alternate history • Utilitarianism • Cascading Style Sheets • CSS (band) • GitHub • Software developer • Computer file • Debian • Open-source model • Software development • Application software • Distributed version control • Workflow • Needing You... • Cascading Style Sheets • Software bug • Real-time computing • Chromium (web browser) • Workflow • Software developer • Branching (version control) • Commit (version control) • Branching (version control) • Software developer • Graphical user interface • Source code • Time • Google Chrome • Software developer • Software developer • Source code • Command-line interface • Source code • Google Chrome • Facebook • Google • Facebook • Gatekeeper (macOS) • Facebook • Facebook • Gatekeeper (macOS) • Facebook • Workflow • Chromium (web browser) • GitHub • GitLab • Code review • Google • Open-source model • Programming tool • Covalent bond • GitLab • GitHub • Server (computing) • User interface • Code review •