In a previous post, I talked about practicing code reviews with empathy. This time I wanted to share three common code review mistakes I made in the past:
Assuming Senior Devs Are Above Code Reviews
Senior devs have mastered the craft, right? After thousands of hours of practice, they've reached the final form--the perfect programmer. But senior devs are humans. Humans make mistakes, and code reviews are the perfect tool for catching mistakes early. Here's Steven McConnell on the effectiveness of code reviews:
… software testing alone has limited effectiveness – the average defect detection rate is only 25 percent for unit testing, 35 percent for function testing, and 45 percent for integration testing. In contrast, the average effectiveness of design and code inspections are 55 and 60 percent.
Now I've learned to treat all code reviews equal. Reviewing a senior dev's code has extra benefits too. I can learn their coding style, their problem-solving techniques, and how they respond to feedback.
Not Participating in Code Reviews
When I was a new hire, I would feel like I couldn't contribute to code reviews. How could I provide feedback without understanding the software architecture? But prior knowledge isn't a requirement for participation.
The trick is to ask questions. "I'm having trouble understanding this change. Could you direct me to any documentation?" Or "How can I learn more about why we went with this approach?" There is no learning without asking questions.
Even when I've built up the understanding, I ask questions. Because a "ship it" means I'm just as responsible. And I don't want to ship something I don't understand.
Assigning The Same Code Reviewer Repeatedly
Once I knew who had more knowledge in a particular system, I would assign him or her to my code reviews repeatedly. This person could give me better feedback, I had thought. I don't know if this was true, but I was contributing to major a problem.
Assigning code reviews to the same person is bad for knowledge distribution. Only one person knew what I was trying to merge. What happens if the person is busy or PTO or hit by a bus?
I try to add more than one person to a code review nowadays. This way there's transparency within the team about what's changing. Everyone has a chance to review my changes and learn. Best of all, having more pairs of eyes helps catch more errors.