Whenever I hear development teams discussing code reviewing, I always bring up a story about the way Porsche used to build their cars. Problems found in code review are nicely analogous to the problems you might have found on your 1990 911, and we might be able learn from the way Porsche changed their focus in order to change things ourselves.
At the beginning of the 1990s Porsche were spending ever-increasing amounts of money on fixing their cars after they had left the production line, but before they reached the customer. As James Womack and Daniel Jones' classic Lean Thinking tells it, the best Porsche engineers were all involved in this process:
In the paint booth, it was accepted that “first-time-through” quality would not be very high due to contamination which was very difficult to eliminate, but that skilled paint specialists could eventually bring the body paint up to an acceptable level. Finally, once the moving track was installed in 1977, the operating philosophy was to quickly put all of the parts on the car, then test them as a system after the car rolled off the line, and to rectify errors in a highly skilled troubleshooting and rework process which eventually produced a product with a world-class low level of defects as reported by customers. Skilled work was, therefore, defined as the ability to operate specific machines and to diagnose anomalous conditions during long work cycles and to take corrective action on a case-by-case basis.
The end result of this was that the cost of making a Porsche was higher than even the very high cost of a Porsche to a customer. Unbelievably, Porsche were losing money on every car they sold.
In Lean terms, Porsche's cost of rework was a waste (or muda). It was one of many wastes and the Lean transformation of Porsche was not just about eliminating this cost. But a welcome side-effect was that rework costs went down while at the same time quality went up. And, in an impressive turnaround, by the end of the decade Porsche was one of the world's most profitable car companies.
Similarly, code review is a rework waste in software development.
Just to be clear, I'm not saying that the act of reviewing code is a waste of time. Just as someone still counts the wheels on your new Porsche and drives it around before it leaves the factory, a human should still inspect all code going through your team with the same high quality standards they always did. I'm also not focusing here on the design review that often happens during code review. This is something which perhaps deserves a post of its own, but the summary is: review your design earlier.
For this post, I'm specifically concerned with code defects found during review. Defects should not be seen as unavoidable, or "just part of the process". Defects are evidence that something is wrong with your system and so you should try to fix the system, not just the defect.
Unfortunately, in my experience engineers spend a considerable amount of time in retrospectives discussing how to make code reviews run more efficiently. Perhaps using one of the popular code review tools might help? Perhaps we just need to be clearer on our code review principles and coding standards? Perhaps we should just be nicer, harsher, looser, more pernickety, or just oscillate wildly between all of these extremes? These may be valid ideas, but they are all focused on optimising the management of defects, rather than eliminating them in the first place.
As managers, the question we should ask is "what could we do in our process to remove the waste in code review altogether?". This would encourage discussion on more fundamental changes; changes we could make to our process that mean that all code arriving in code review is free of quality issues and passes the human eyeball check every time.
How to do this will depend on the specifics of the process, but there are well-known techniques that can be used to fix the systemic problems commonly exposed by code review. Pairing can catch code issues upfront as the code is written, shortening the feedback loop between author and reviewer and reducing the cost of rework. Coding standards can be enforced automatically with static code analysis, meaning trivial style errors are fixed at source instead of wasting review cycles. And (my favourite), reduced batch sizes, in the form of smaller commits, mean far less cognitive overhead for both coder and reviewer and a reduced chance of cascading changes required from a single problem.
So, next time a retrospective strays into fixing code reviews, it might be time to try and turn it into a discussion about fixing quality.
- More about Porsche's transformation to Lean: http://cavqm.blogspot.co.uk/2013/06/porsche-lean-manufacturing-and.html