Pull requests are great
I have recently been seeing an increasing amount of chatter about and against pull requests. These especially often come from the crowd that advocates for pair or mob/ensemble programming. I saw a great one in Mastodon the other week but failed to save it so I can’t reference it. In essence, that toot asked: What legit benefit is there for pull requests for teams that trust each other?
And I’ve seen this sentiment quite often: some people consider that pull requests’ main or even only function is to prevent malicious or bad code from entering a codebase from untrusted sources. And in many distributed open source projects that is one of its functions. However, I’d argue that focusing solely on the trust issue and then dismissing pull requests for teams that trust each other, is short-sighted.
I’m a fan of pair and ensemble programming and this is not a blog post dismissing or refuting them. Often pair or ensemble programming end up being the answer to many of the benefits of pull requests and for some cases, I agree. I’m writing this post from the perspective of a team that doesn’t do those as most teams don’t.
Some people, like Gregor Riegler in his article Solo Programming Considered Harmful (it’s a great read!) consider working on features alone (within a team) a bad thing:
It turns out that solo programming may not be as good as we think. No, I’m serious. I think we overestimate our programming skills. We tend to overcomplicate things, and we make a lot of mistakes. We rarely understand the problem that needs to be solved, and we don’t know our tools well. We often end up with crufty code that isn’t as simple as it could be. It doesn’t work and often targets the wrong problem.
These people have a point but I feel they often throw ensemble programming as a one-size fits all solution and silver bullet without trade-offs worth considering.
One thing that pull requests (and solo programming) enable is asynchronous development.
Personally, I value flexibility in work a lot. Whether the teams is spread across time zones or located in the same city, people can work at different times of the day effectively. I’m currently doing 80% work week meaning I’m out of the office on Wednesdays. That means I can wrap something up and create a pull request on Tuesday and there’s some time for others to review my work without having to wait for me to return.
Or if I’m working on European timezones and my team mate works in west coast US, we can bounce off ideas and code reviews when the other one is away.
Asynchronous communication isn’t easy and it requires putting in the work to learn and become effective with it but it can also open a lot of opportunities.
In a good team, I find pull requests more important for shared learning than preventing bad code from entering the codebase. If you ensemble everything with the whole team, that can work too for learning but it’s hardly a reality for most teams, even to those that do some part of their work in groups.
Pull requests and code reviews are a great opportunity for the team to share understanding of the codebase and how it’s evolving. They are often seen as a “senior developer reviews junior’s code” but it can be so much more. Code reviews on pull requests should not be seen as a judgement of an effort but a (well crafted) starting point for a further discussion. Some part of the discussion often needs to happen before decisions are made all the way to code level but there’s still room for them in the pull requests.
I often read through my colleagues pull requests even if I’m not per-se reviewing them as it helps me see what’s happening, to learn from their approaches and style of solving problems and how they structure their code.
It’s a unit of work I can go back to
Even if I was in a team that ensembles, I’d still like to wrap certain units of work into a pull request. I do that all the time when I work alone on projects: my changes go to pull requests and I sometimes even let them sit there for a moment and then review my own code with fresh eyes.
Unless the entire session or feature is worked into a single commit, I find it valuable to be able to go back and look at the development as a unit that can be easily found from a pull request.
Recently, as a new person in the team, I’ve implemented multiple features where there’s been a previous similar approach used in another feature. Being able to open an old pull request with well crafted step-by-step commits helps me a lot in understanding what goes into building this feature and I’m able to see what parts are relevant to mine and what are not. I can even see the comments in the review and the process that it has gone through.
Written code reviews are documentation
One thing that grinds my gears is silent knowledge in teams and organizations. Teams change, people move on to other projects or companies and new people join teams.
I’m a firmly believer that code reviews, even if they’re done in-person with someone, should be written down as documentation to pull requests. Decisions made (even if they deliberately lead to non-action) are valuable information for the future moments and having those discussions written down ensures they are saved for the future.
While most documentation we want to keep up to date to be valuable, there’s value in documentation thoughts, reviews and decisions as-is, frozen in time and version control is a great tool for this.
Sign up for Syntax Error, a monthly newsletter that helps developers turn a stressful debugging situation into a joyful exploration.