Pull Requests are dragging you down

Pull Requests are dragging you down

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

  • Pull Requests (PRs) originated for controlled Open Source collaboration, but asynchronous nature can lead to delays and inefficiencies.
  • Face-to-face or video-call reviews offer synchronous alternatives, minimizing delays and enhancing communication.
  • Pair programming proves effective for real-time code reviews, improving code quality and productivity.
  • In consultancy projects, trust-building with clients helps transition from PRs to synchronous reviews and merge privileges.
  • Synchronous approaches align better with agile development and bolster collaboration.

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:

  • Time-box the reviewing process
  • Avoid typical misunderstandings that happen on comment exchanges
  • Be a better learning experience

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:

  • Improved code quality, due to constant reviewing
  • Improved productivity due to increase in focus
  • Improved learning experience

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:

  1. Make sure to understand the client’s code stack, standards and code base. If it helps, create documentation, diagrams, etc.. This will also help your new team members to be onboarded into the project
  2. Start suggesting face to face code reviews or, if possible, pair programming sessions
  3. Gain their trust: while doing this, take note of the typical comments your client makes, so that you avoid these errors in the future and start proving that there is more adherence to their standards by the reduction in defects
  4. Once you’ve got their trust, start by requesting approving privileges for small or urgent fixes
  5. After that, the client should be comfortable with you having merge privileges for the team’s code

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.

要查看或添加评论,请登录

Matias Salerno的更多文章

社区洞察

其他会员也浏览了