Code review as team performance indicator
Sometimes people wonder what is a good way to measure software development team performance. While the obvious approach is to focus on the results, the outcome, I suggest that code reviews is another thing you should pay attention to. Personally I have seen that in the best teams code reviews work naturally, bringing a lot of benefits that enable creating valuable outcomes. In the less performing teams code reviews are skipped or they are somehow forced.
Code reviews can bring at least the following benefits. They increase the quality of the code. They act as a gate both for internal and external quality. They spread information inside of the team and that way support shared code ownership. They amplify learning. For the new team members they are one way to get along with the features of the product and the way of working in the team.
So if there are so many benefits for doing code reviews, why aren't all teams doing them? Here we come to the point why they measure the team performance. In the best teams, in my personal experience, people understand the benefits and they are willing to review others' code. They appreciate their own and others' learning. They want to have truly shared codebase instead of functional or component coding silos. They see how code reviews increase the quality of their work. If I summarise this into one sentence - they care. A team cannot perform very well if the team members don't care about these kinds of things.
The problem with pull requests
Since the very beginning my current team has done all the code reviews face-to-face; the author explains commit by commit what he has done and why. However, a couple of months ago we got new team members that are not located in Helsinki. This led us to the situation where we started using pull requests. When a remote team member finishes something, he creates a pull request and someone from the Helsinki site reviews it. We are using Stash where you can leave comments to any code row and have a conversation there if needed. Works nicely, kind of...
Little by little I got annoyed when I realised how much time it takes from me to do a code review. First of all, when there was just me reading the commits, it felt really inefficient. I had to make guesses what the author had meant and since I wanted a confirmation, I needed to write a comment. This led to another inefficiency, conversation around commits. Then there usually was another round of commits based on the feedback and sometimes similar problems also on this round. If my initial feedback was misunderstood, the new commits created even more waste to the process.
Of course in some cases I took the conversation off from the tool and we used a chat. But all in all, even though we had a nice tool, this wasn't working well. From the lead time perspective, because of all the delays in the process, it was really slow. From my personal perspective, it was frustrating and took time from other things.
However, we didn't change anything for a while. Maybe all of this wasn't so clear to me at that time. Maybe I was too busy to hop on the bike.
Then at some stage the remote team members suggested that the Helsinki programmers should start doing pull requests as well. That way they could learn from us and understand better what we are doing. And I think that there was also an equality aspect between the sites. My intuition was against this idea but I couldn't reason it at that time. I remember mentioning something about the pull request tool and one of the remote team members replying: "it's a great tool". And yes, it is a great tool, it is just solving the problem in the wrong way.
Because it would have not been fair to judge the suggestion beforehand, we tried it. One of the Helsinki team members created a pull request for the remote team. They reviewed it, gave some nice feedback and he did the modifications based on that. So in a way it worked pretty ok. Expect that also in this case there was a bit of the delay issue but still.
Get rid of pull requests, start talking to each other!
The most valuable feedback from this experiment was that it made me think. I felt that despite of the good intentions, we are going to the wrong way. It's not that Helsinki people should start doing pull requests but the remote people should stop doing them. Or at least we should change the way how we go through the pull requests.
When I started doing yet another code review and during the first two commits I already had a couple of questions, I just decided to stop there. We went to Google Hangouts and did the code review just like we had been doing with the co-located team members: the programmer goes through the commits one-by-one and explains what he has done and why. And most of all, we discuss!
It's probably not difficult to guess how it went. I got immediate answers to my questions. We got some short discussions about other commits too. I was able to give some feedback so that I was confident that he understood it. Code review was quickly done and we both felt it was exactly how we should do them also after this.
And this is how we have continued after that. There have been some small code reviews done without talking but most of them we have done by discussing. The benefits I just mentioned have realised pretty much every time and besides them there have been other ones too. Sometimes code reviews have generated discussion about future plans or ideas how to improve our code in general. In some cases the good discussion has saved us time because we have concluded for not doing something. And of course there is the rubber duck effect; quite often it's not the reviewer who realises something but the author himself. This is something that could not happen without people talking to each other.
Summary
Programmers tend to be people who enjoy using technical tools. From that point of view it's easy to understand why a well-built pull request tool may sound like a great way to do code reviews. However, by doing code reviews by discussing you get so many more benefits compared to code reviews where the reviewer just reads the code. No matter how good your tool is, it cannot replace people talking live. Whether it is face-to-face, by phone, with a video tool, or some other way.
No comments:
Post a Comment