Pull Requests are dragging you down
Matias Salerno
Tech Lead @ Macro Securities | Speaker | YouTube: @programmingwithmati
Pull requests are by far the most common way to do Code Reviews. But they are BAD. Really bad. They are destroying your team’s productivity.
TL;DR
Why Pull Requests?
PRs were born out of necessity. They were introduced for the purpose of gate-keeping changes in Open Source projects. This makes sense. Open Source projects are managed by multiple collaborators who often are not even located in the same country/time-zone and probably don’t even know each other personally. Many new collaborators can appear and the maintaining team is right in monitoring the changes. You don’t want new collaborators to introduce bugs or security breaches into your Open-Source project.
So, even though PRs are slow, they are the only alternative between not allowing people to contribute and letting anyone commit whatever they want into the project.
Why are PRs dragging you down?
What is your objective as a software developer? To write code that accomplishes a particular functionality. But software development is not simple. There are so many alternatives and possible scenarios that we need to handle, that the best way to approach our solution is to have many feedback loops that will tell us if what we did is correct or not.
For example, I am a huge advocate of TDD and that helps me get instant feedback loops on my logic, so that I can tune and correct anything that is not behaving properly.
But after you test your code, you need to make sure that it works with everyone else’s changes and to do that, you need to merge your changes as soon as possible. Here is where the PR is crushing your productivity. They are really slow.
PRs are asynchronous
PRs are an asynchronous process. You post your code for someone to review. Then you need to wait for their comments/change suggestions. And then an asynchronous exchange of messages and new code happens, until the reviewer is happy with the code and finally merges it.
Because of the asynchronous nature of the process, there are delays. I’ve seen this process go on for days, even weeks.
Our attention is an extremely limited resource, much more now that we have so many things competing for it. Imagine you are working on your own task, really focused, and someone sends you a PR for you to review. It will probably sit there until you manage to at least take a look at your slack/email (who uses email now?).
Then you look at the PR and you are lacking context, you go to Jira, open the ticket to remember what it was about. Then start reviewing, and there are things that are not clear to you, so you put a bunch of comments. Then the developer, who is trying to start a different task, again needs to stop focusing on that new task to solve those PR comments. He doesn’t understand some of them. He doesn’t agree with others. You see how inefficient this whole process is?
Multiply this for let’s say 2 senior devs who have to review the PRs of 4 or 5 team members. What a drag.
Make Code Reviews Synchronous Again
Code reviews don’t have to be asynchronous! We have gotten so used to the PR method, that it seems we can’t think of a different way of doing a code review.
领英推荐
Doing face to face code reviews may seem like a lot of effort and time wasted, but actually it will be way more efficient than what we described with the pull request method.
If you are working in the same office, next to each other, then there is really no excuse. And if you are working remotely, then do it on video-call. Share screen. It’s so easy.
This will:
Make code reviews part of the process of coding
Why not take it a step further. By now we can agree that coding seems to be a 2 person job. One coding and one reviewing. So why not incorporate the reviewing in the coding itself. Pair-programming is excellent. It has so many benefits, like:
There is a lot to talk about pair-programming, but the main focus here is the improved code quality. This is due to having someone who is constantly reviewing the code as it is being created. The paper “Strengthening the case for Pair Programming” by Williams, Kessler, Cunningham and Jeffries ran an experiment giving assignments to university students. Some of the students worked individually and some in pairs. They had to develop four programs in the period of six weeks.
The result was that the pairs passed 10 to 20% more of the automated test cases compared to the individuals.
Not only that, but the pairs submitted their solutions 40 and 50% faster than the individuals. Not taking into account that individuals would have taken even more time if they would have been asked to improve their solution to increase the amount of test cases passed.
This is just one example, but there is more research backing up the case for pair programming.
I know, pair programming sounds scary. Project managers can be skeptical of its efficiency. Developers that haven’t tried it may be concerned about working in this way. Although research shows that more than 90% of the developers that did try pair programming have said that they enjoyed it more than working individually and they feel more confident in their solution.
Working as a contractor
All my professional life I’ve worked in software consultancy. This means that a company hires us for developing a particular project that they will own. This often requires you to adopt the client’s standards and good practices for coding. In the end, you are modifying their codebase. Now, initially there may be a lack of trust which is understandable. Their coding standards are new to you, and you need to learn them. That’s why, in many cases, clients will have a technical point of contact who will do the code reviews, making sure you adhere to their standards.
This is fine, but the problem is that they will probably do code reviews using pull requests. And this will slow down your team. Which in turn makes the client think that your team lacks productivity. To tackle this problem, I use this approach:
This is really important. If you and your client are doing agile development, there has to be an increase in trust. The client has to be involved in the project and be part of the team. If there is no trust, then working together will be really hard. Always build trust with your team, especially with your client.
Conclusion
Pull requests are a model adopted from Open Source projects which is not ideal for an agile project team. In my experience, face to face code reviews and pair programming have worked better than PRs. If you are working as a contractor, then build trust with your client, so that they feel comfortable with you merging code into their code base. Prove that you can learn their standards and that you are an excellent software engineer.