It has always interested me to see how different development teams treat code reviews. Some teams see code reviews as crucial to their success while others see them as a barrier to getting things done. Over time, I’ve come to the conclusion that the culture around code reviews is an important indicator in predicting outcomes for a team.
I wholeheartedly believe this because I’ve seen the quality of code (and thus the product) and the culture of the team directly affected by a team’s code review practices. Ultimately, we can observe a team’s genuine agility in how it handles code review.
The Starting Stance for Code Review
The attitude and expectations with which one enters an activity, like reviewing code, is a starting stance. The starting stance of everyone involved in a code review can greatly affect the outcome. The starting stance can be the difference between learning from each other versus building resentment for negative interactions.
The most effective starting stance for reviewers and reviewees alike is one in which there are no pre-conceived ideas about what will be found or what will be concluded. Starting each code review with a blank slate of expectations is the most effective starting stance for everyone, opening the door for anyone to learn from anyone else regardless of rank or expertise.
One Team’s Code Review Journey
I had the privilege of working with a team for a period of time during which it matured its code review practice from informal to very stringent, using code review as a prime means of communication among the team members. The key to their success was that no matter how stringent code reviews became, the entire team was dedicated to improving them and was committed to frequent and early inspection.
The informal over-the-shoulder code review is popular among many agile teams and can demonstrably improve code quality without the full pairing experience. This is a great entry point for effective code reviews, and this simple practice can go a long way toward improving code quality and collective code ownership, a widely respected core value of Extreme Programming.
This team used the over-the-shoulder technique to great effect. They started with a simple rule: Explain your code changes to someone else before you check in.
However there are drawbacks to over-the-shoulder code reviews, too. By definition, it requires another developer to stop what he/she is doing and come review the other developer’s code. This break in concentration interrupts a developer’s flow as much as going to a meeting. It requires time for the reviewer to get back into his own flow, and is essentially a task switch from which the reviewer must recover.
Further, not everyone on the team got to see the changes. Only two or three people would see the changes flowing into the code, resulting in surprises about unexpected change for the other team members. Also, this practice didn’t guarantee the right people were reviewing the code. Sometimes the people who were most needed for a review, or who would be most impacted by a given check-in, weren’t available and this created bottlenecks that left developers waiting for someone to return to the team room before they could proceed with their check-in.
That team eventually implemented a “gating check-in” model of code review using a more sophisticated code review tool. The tool allowed the committer to stage his code for review before it was merged into the team’s source control development branch.
In this model, the developer submits code to a version control branch, or other staging location, and requests a code review from a named set of reviewers. Reviewers might include the entire team, or even other developers on teams that might be impacted by the coming change. Upon issuing the code review request, potential reviewers were notified in various ways, depending on how they had subscribed to code review requests.
Once the code review request was issued, it might need to meet certain criteria to be complete, after which the code was merged into the team’s source control. Criteria for a completed code review might include such items as:
- The code must be reviewed by at least two other developers.
- The coming changes must be seen by one or more specific individuals.
- The code review must include comments by another developer.
- The code must be reviewed by a member of another specific team.
This might all sound rather onerous and not very agile. This is where the team’s starting stance and behaviors come into play. If check-ins and code review requests are issued frequently, it becomes the norm to review code often. Indeed, it became the norm that anyone waiting for a code review request should themselves be reading code from another requester. This raised the collective awareness of everyone on the team about what was changing and happening in the code, thereby promoting collective ownership rather than detracting from it.
With a commitment to frequent and early inspection, it became normal for everyone on the team to commit code and request a review about twice a day. This may seem high or low to you depending on your stance, but it worked very well for this particular team. Everyone spent as much or more time reading code than writing it and this meant that developers learned from each other every day, while keeping a basic understanding of how their product was constantly changing.
You might think the team’s throughput suffered, but it didn’t. In fact, the team’s productivity increased because frequent code inspection and resulting discussions kept a very high bar for quality in the product, which meant features were easier to add.
Being agile doesn’t mean not being thorough, and a thorough code review process can be used to promote team agility rather than detract from it. I have been lucky enough to see this in person and am a firm believer in the power of code reviews to enable agility and shared code ownership.
Finally, code review can be a primary learning tool if everyone involved approaches it with a sincere starting stance. Regardless of technique, code review is a powerful practice that should accompany any software as it flows through its journey toward production.