Pull Requests in the Workplace

I've worked with quite a few companies now - both as a permanent employee and as a contractor/consultant. However, up until now, I haven't worked with a team who use Pull Requests for all code changes. Whilst I know the PR model well through GitHub and open source - I hadn't previously used it within an actual workplace.

A couple of years ago, I tweeted this...

Reading a conversation on a @CodingBlocks Slack channel about using Git pull requests as part of a team's workflow. How common is this?

— Dan Clarke (@dracan) April 21, 2016

It struck me as quite an interesting idea at the time - and I could imagine both pros and the cons of doing this. After now experiencing it within a team, I thought it would make an interesting blog post to delve into these pros and cons.

The TL;DR; version is that I will be highly recommending using this in teams I work with in the future.

Pros and Cons

So the pros and cons are quite obvious - but let's list them out anyway...

Pros:

• Code cannot get into the main branch without someone else reviewing the changes, which helps protect against bugs and defects.
• It's helps with the Bus Factor - ie. knowledge is shared and not solely known by just one person.
• Forces team communication and prevents developers working in one-person silos.
• Promotes mentoring and learning between team members.
• Code and design patterns becomes more consistent.

Cons:

• Each change takes a lot longer and there's more friction to getting a change into the main branch.
• Each change requires interrupting another developer and taking them away from their own work.

Are these really cons? Let's look at the bigger picture

Okay - so now let's look at the reality of these cons. In the team I'm working with who use this PR model - the vast majority of PRs require at least one change after the PR review. Quite often multiple changes. This does not mean that the developer did a bad job - we're all only human after all - but it does show the value and importance of code reviews. Having someone else look at changes from a different perspective and with different experience and knowledge of the codebase can make a massive different to a codebase.

Given that most of the PRs require code changes of some form - if the team wasn't using PRs - then those changes, improvements, and bug fixes WOULD NOT HAVE BEEN MADE!!

I'm going to quote a few paragraphs from a blog post I wrote a couple of years ago: "Automated Testing Series - Part 1 - What and Why?" ...

A bug found during development doesn't tend to cost a huge amount of time.

If that bug makes it to QA, then it costs much more time, as more people have to get involved. The tester who found the bug has to report it and also communicate how to reproduce it to the developer. The developer then needs to try and reproduce it, fix it, redeploy it to the test environment. The tester needs to then retest. Sometimes this can go back and forth many times.

If this bug isn't caught by QA and makes it to production, then we're now involving customers, testers, project managers in that cycle. A simple bug that would have initially taken a few minutes to fix has now potentially cost us many days and has involved many more people. Not only this, but we've risked losing customer confidence at this point.

Think about the time saving if we catch most of our bugs before they even leave the developers' machine and get committed to source control. To say it's HUGE is a bit of an understatement.

Whilst that quote refers to avoiding bugs with automated testing - exactly the same applies with code reviews. If bugs can be caught at development time rather than in production - the time saving is HUGE. This really puts into context the "con" I listed earlier about each change taking a bit longer.

Remember - time spent today can save a lot of time tomorrow!

Knowledge Sharing and the Bus Factor

Another pro I mentioned above was knowledge sharing. If you haven't heard the term bus factor before - it basically refers to a single person having knowledge that hasn't been shared. What happens if they get run over by a bus? Obviously the likelihood of it being an actual bus is quite slim - but people leave companies, people take holidays, people get sick - you get the idea. Having a single point of failure like this can be quite dangerous for a company / team. Using PRs for code changes, help mitigate against this.

It also helps with mentoring. Different developers have different experiences with different technologies, design patterns, paradigm, ideas, etc. Opening up discussions about code changes, can really everyone learn from their peers.

What about pair programming instead?

A lot of the pros and cons I've discussed above also apply to pair-programming. You're using twice the resources for the same task - but this certainly is not a 100% loss of resource. Two brains solve problems faster, spot more bugs, have a larger range of ideas. You also have the benefit of it being harder to procrastinate when you're not working alone.

So I'm certainly not saying PRs over pair-programming. In fact, PRs quite often become in-person discussions. A great thing about PRs though is that you have a history - and the tooling can enforce this workflow.

UPDATE (2020-06-20): I've just written a blog post about my recent experiences pairing full-time at a new contract. I feel that this takes the benefits I've described in this PR blog post to the next level! https://www.danclarke.com/2020-pair-programming

Please retweet if you enjoyed this post ...

Blogged: "Pull Requests in the Workplace" https://t.co/1uRwtHZDJk #git #pullrequests pic.twitter.com/Zl3WEfYIPb

— Dan Clarke (@dracan) November 10, 2018