The one code review method to rule them all
Find the method that works for you, and don't let anyone guilt you into thinking your way is "wrong".One thing most of us can agree on, and for once the data seems to support us, is that code review is good for us.
What we can’t seem to agree on is how to do code reviews. There seem to be two basic camps these days; those who prefer either:
- Peer Programming
- Pull Requests
So which is the one true way?
There are some pretty strong arguments in favor of peer programming. This is by no means an exhaustive list:
- Real-time knowledge and context sharing
- Excellent mentoring opportunity when you pair a more senior dev with a less senior one
- No lag, waiting for code review in the PR model
Those sounds pretty compelling. So what are the arguments for pull requests? Also not an exhaustive list:
- They don’t require a synchronized schedule
- Avoids the potential problem of the code being reviewed by a peer who is “too close” to see the flaws
- Easily allows for more sets of eyes, when appropriate, which can be beneficial for mentoring
So both methods appear to have some benefits, and some drawbacks. So which method should you use?
The one true code review method is…
… drum roll, please …
the method that optimizes for what your team and situation most needs!
If fast throughput is the most important thing on your team, and you have enough engineers working concurrently, and you don’t care that your code may suffer by not being reviewed by someone who’s hasn’t been staring at the code for 8 hours, then using peer programming in lieu of pull requests is probably the right choice.
On the other hand, if your team often works asynchronously, or you value extra eyes on some of your code for the sake of either mentoring or code improvement, or you aren’t (yet?) comfortable with pair programming, then maybe pull requests are the best bet.
Of course, you can also do both. I once worked on a team that agreed that every line of code should be reviewed before merge, but that if the code was written during a pair programming session, that counted, and the PR could be merged immediately.
TL;DR; Find a method that works for you, and don’t let anyone guilt you into thinking your way is “wrong”, if it works.