Code reviews are very important. They are the last line of defense. Code review is the last chance we have to stop a problem from going out into the wild. Whatever that problem is, whether it’s a bug hiding in the bushes, or in plain sight. Or a problem that will make our life harder in the next sprints, and we find about it now.
And of course, it’s the last chance to stop people from putting the braces in the wrong place.
Code reviews are so helpful. And that’s why everyone loves them!
Well, almost everyone.
Because almost everyone loves them so much, we do what we always do for things we love: We give them the automation treatment! Part of the process can be completely automatic – with linters and static code analysis. For the rest – we still need a human.
But we don’t like that human, in our case our great team leader, to be disturbed. Especially while she’s trying to solve a great technical mystery. We’ll just ask her to pull our code and review it when it’s convenient for her. Later.
While we’re waiting for her feedback, we can surely move on to something else. At least until she gets back to us, with her comments. That would be in an electronic form, of course, not actual talking, as we don’t like talking anymore. And then, when it’s convenient for us, we’ll work on the corrections. Maybe we’ll throw the ball back to her to review those corrections too. Then we’ll finally release the code to freedom.
Nobody’s bothering anybody else. Everyone’s doing things in their own time.
We even got a shorthand term for that – “pull request” . It’s not the original meaning of a “pull request”, but everyone hears “code review” when you say it.
There’s a couple of little snags with this upgraded automated process, though.
The Code Review Slows Us Down!
Well, they shouldn’t, but this process does.
First, and most importantly, the road to “done” gets longer. We add wait states in the process. Instead of reviewing and releasing, we need to wait for it to be convenient for everyone. Step after step.
Next, there’s no real learning. Learning is usually done through dialog, discussion, sometime arguments. But since we sent the writer and reviewer to the opposite corners of the ring , there’s no real communication. No knowledge is being developed or passed.
Remember, we thought code reviews can help us with quality? This process doesn’t really help improve quality. It almost works against it.
Because of the delays, when it does become convenient for the team lead, does she really go into the details of the code? Not really, she has other tasks too.
She knows that the review of the code is important, and requires feedback, so we can merge it, and deliver it to production. But on the other hand, she really wants to go home, it’s almost 8pm.
So, how well would she review it? She’ll go over the easy-to-find stuff, and probably the easy-to-fix stuff. And hopefully – the really important stuff, if she notices it in time. Code reviewed like that, will not improve quality much.
And when we get back the feedback, we’ve already moved on three tasks from where we were. We don’t remember much about the code (except for how relieved we were to pass it to review).
We consider the feedback, fix the easy stuff, and if we don’t have tests, maybe not even that, because it’s risky. And then we merge and push.
The more delays we have in the process, there is less improvement in the code, and with the people. Also, within this process, because we like our team lead so much, we may batch as much code as possible, so she can review all of it in one shot.
She thinks it’s nice of us too, until she looks at all that code she needs to review now at 7:59. Not much good will comes out of it.
What’s the best way to do code reviews? Sit together. Talk and discuss. And if you can, do it a couple of time a day.
Wait, Wait, Listen Up.
How much code can you write in a couple of hours? Not a lot, right?
Now think, how long do you think you need to review that code? Enough to warrant a “Hey TeamLead, can you look at this code, and tell me if we’re good?”.
These couple of minutes, every once in a while, speed things up. No need to schedule a week upfront. And if you really want group learning, it’s OK to bother your teammates, not just the almighty team leader.
But if you want to get rid of code reviews (but keep quality up), there’s a sneakier way. It’s called Pair Programming. Which sometimes gets called “extreme programming”. Again, not what the original term meant, but that fight is for another day.
The reason it works, is that when two (or more) people work on the same code, they review the code all that time. When it’s done, at least two people signed off on it. They both move to other tasks, and that’s really “done”.
If you don’t like pairing, I suggest going with the couple-of-times-a-day review. I used to do pair programming as much as I could, but when I couldn’t this came in second. It works, less stressful, and everyone can spare a couple of minutes on their way to get coffee.
Just don’t do it at 8pm, please.
But do check my Clean Code workshop, where we discuss code reviews in detail.
0 Comments