Pretty much everyone does code reviews. They’ve been around a long time. I remember back in my Borland days when the Chief Scientist would come in every morning and review all the code that had been checked into the Subversion(!) repository the previous day and send emails out to folks whose code wasn’t up to snuff. That’s old school.
Slightly less old school? Saving all the check-ins up until Friday for the Dev Leads and/or Dev Managers to review and approve. Both of these techniques leave a lot to be desired -- the main thing being a complete lack of interaction between the developer, the code, and the reviewer.
Code Reviews have a number of purposes. Probably the most important one is preserving the quality and integrity of the code in the repository. Even the two old-school ways above do that.
But almost as important is the learning opportunity that code reviews can provide. If the only feedback a developer gets from a code review is mistakes in formatting or other trivial things like that, then nobody learns and gets better. The old school ways above provide for few opportunities for a developer to increase their skills.
To provide learning opportunities, code reviews evolved into meetings where everyone looked at the code written that week and commented on it, criticized it, or otherwise ran it through the gauntlet. This did provide a learning opportunity for developers, but it took more time, as it was 100% synchronous and required all code to wait for the next scheduled meeting to be reviewed.
Now, almost no one is doing these old-school code reviews anymore. All the cool kids are doing pull requests. (Some folks call them “merge requests.”) Pull requests have a number of advantages over the previously mentioned methods, including:
Interestingly, some say no, you shouldn’t.
Not only is Jessica Kerr a great speaker and a good Twitter follow, but she also has some interesting ideas about code reviews in her article of March 27 entitled "Those pesky pull request reviews." In fact, she doesn’t like pull requests, and argues that you should sidestep them by just working on a given task as a team, so that everyone sees everything as the work gets done.
She believes that pull requests work great for open source projects where a “team” is really a set of individuals coordinating work together. For true development teams, she believes that if a team all works together on a single task, everyone learns and understands the code, and thus there is no task switching between coding and doing pull requests because the pull requests are unnecessary.
Jessica’s idea is radical -- basically going beyond Pair Programming and moving into mob programming. Mob programming is the idea of having whole teams work together on projects in serial rather than individually in parallel. Mob programming can eliminate the need for pull requests by causing all of the communication and learning to take place during the coding phase, without any review.
I’m having a hard time agreeing with her idea for a couple of reasons:
As part of a discussion about code reviews, Rob Kraft, one of the Development Leaders in our vibrant Dev Interrupted Discord Server (you should join!) made the following comment that I agree with:
I think that what Jessica needs is a good look at LinearB. 🙂
Let me address some of her more specific objections:
Okay -- so what rubber is hitting the road here?
If pull requests and code reviews are hard and people don’t want to do them, then you are doing them wrong. So the trick is to make them easy to do.
We here at LinearB see many, many customers improve their Cycle Time and their overall software development process by using and tracking pull requests. By combining metrics tracking around pull requests with tools like WorkerB, many, many development organizations have seen smaller pull requests, better reviews, shorter Cycle Times, and an overall sense that things are really humming.
Monitoring things like the size of pull requests, when pull requests are assigned, picked up, and commented on, as well as monitoring the depth of reviews that take place all create an environment of small, discrete, easy to review pull requests.
And of course, if you want to find out more about what our customers already know, you can book a free demo of LinearB.
In the end, while her ideas are intriguing and thought-provoking, I can’t say I agree with Jessica’s argument. There doesn’t seem to be any good reason not to do pull requests with code reviews.