A Value System for Code Reviews, and How to Scale Them for a Large Project
Thursday, September 18, 2014
Roy Osherove in First Steps during chaos, Growing People
One of my customers wanted my help is increasing their code quality.
When I spoke with the developers the following conversation happened several times:
Me: "Wow, this code... how many lines long is this method?"
Dev: "about 700. Yeah I know"
Me: "Have you tried to refactor some of it?"
Dev: "oh.. noooo."
Me: "What if we just extract this small part over here , these couple of lines, to a method with a good name?"
Dev: "Yeah, let's not. If I do that then I have to do a code review on that, and that can take 24 hours to 3 days. The architects are so busy.. too much of a big deal. I'd rather focus on what I'm doing"
When I went to the architects I heard from them how overworked they were. They meant well, but 2 architects on 30 devs, with more of their own stuff to do as well.. it just didn't scale.
I went to the chief Architect and told him what I saw. I told him I felt that the way the code review process worked (reviews are done through a tool, and take very long) is actually deterring people from changing the code for the better. He was surprised to hear it. After all, they had instituted mandatory code reviews to increase code quality. So we talked about what we can do.
Here's the plan we setup in place:
We will organize a team meeting of all the architect where we will agree on some basic values of what we want from code reviews. I noted that the following values worked for me:
A code review is successful if:
- The developer who wrote the code learned something new (even if small)
- The code ends up being better (even if slightly, even if it's just one character or a name change)
- To achieve this, an effective code review has to be in person, not through a tool if possible. if not, then through a non text tool (visual+ audio)
- It also has to occur quickly , or even immediately when the code is ready for check in
- It increases the amount of people that can review code, or the amount of code they can review well
We decided to experiment for a couple of months with doing reviews this way:
- When a developer needs a code review, they can just go to an architect and ask for it in person (not using a tool hopefully).
- The architect will join the dev and review the code, while talking as much as possible about their thinking process, coaching in the process
- A THIRD person will join (another dev) the review, sitting near and learning how to review the code.
- The next time a review is requested, the learning dev (third person) will try and do the review while the architect sits back and sees how it goes, then goes over any missing things with the reviewer
- Developers will check in early and often so that code reviews are nice and short (a few minutes hopefully)
In this way we:
- Make the reviews more effective since they are in person
- Less bureaucratic since they happen very quickly
- This also makes developers less afraid to change code and get feedback on it since it doesn't take a long time
- We make more developers able to be code reviewers, thus making the code review process more scalable. Anyone can review anyone's code within a couple of months.
I wrote more about code reviews in the face of horrible code quality here.
Article originally appeared on Elastic Leadership (http://5whys.com/).
See website for complete article licensing information.