Community

Code Review is an Architectural Necessity: What is it and what problems does it solve?

Share this post:

This post is the first in a series on code review and architecture.

My experience with review

I’ve conducted reviews and had my work reviewed since before I knew reviews existed in software, which is my primary field.

The Holcad of Westminster College, the student newspaper My first experience with review was with the student newspaper at Westminster College. First as a copyeditor, I took on layout, picked up a journalism minor, and eventually rose to be editor-in-chief, despite being a computer science major. For three and a half years, I read everything that was fit to print and a whole lot more that wasn’t. Even as editor, nothing that I wrote went into the newspaper without someone else editing it. No one was exempt from the dreaded red pen, or, in these modern days, word processors’ “Record Changes” feature.

That review process saved us a few times. Sometimes a student writer would completely wreck a piece, but a copyeditor wouldn’t reject it outright. We on the layout team would have to rewrite it on the spot, hours before our print deadline. Just like versioning commit logs, we’d add our name to the byline or sometimes replace it entirely.

When constructing our layout, we’d use lots of X characters as easily-spotted placeholders. It wasn’t until final review that we’d catch them sometimes. One time, a headline “WHAT THE (blank) IS THIS (blank)?” made it to pre-press. My pre-press operator was an amazing woman with an incredible eye for errors. She was nearing retirement and probably had been working in production longer than I’d been alive. She called me and said, “Colin, look at A-8. You owe me a beer,” and hung up.

What is code review?

I define code review as this:

“Code review is the process by which those who maintain a software codebase evaluate a proposed change to that codebase, regardless of the source of the proposed change.”

Wikipedia’s definition is more succinct:

“Code review is systematic examination of computer source code.”

This definition allows for some broader techniques than those I care to participate in (or suffer from), but the similarity is bolded: the act is an examination, and that examination must be systematic.

You may be familiar with the term “peer review,” popularized by Karl Wiegers in his work “Peer Reviews in Software.” I dislike calling the process “peer review” as Wiegers and others do, because that puts too much focus on the peer whose work is under review. By calling it code review, we specify in our nomenclature that we’re reviewing code, not a person.

Moreover, these days, everything is code. Architecture and design documents can be expressed in code. You set up your systems with configuration management software, right? Infrastructure is code. A typo could be an immediate tenfold cost increase.

What and who of a code review

This sentence captures the essence of code review: The submitter proposes changes in a submission, which is evaluated by a reviewer who annotates or accepts it.

  • Change — An individual unit of work altering what exists.
  • Submission — A collection of changes.
  • Submitter — The person proposing the submission.
  • Reviewer — The people evaluating the submission.
  • Annotation — Remarks or ratings bestowed upon the submission.

Karl Wiegers describes this peer review formality spectrum in “Peer Reviews in Software,” where inspection is the most formal and ad-hoc reviews are the least formal.

Wiegers' peer review formality spectrum chart, in order from most formal to least formal: inspection, team review, walkthrough, pair programming, peer desk check or passaround, ad-hoc review.

My concept of code review, and my team’s practice of code review, falls somewhere between “team review” and “pair programming.”

Peer Reviews Formality Spectrum chart with arrow pointing between walkthrough and pair programming, with inspection and team review crossed out.

From time to time, we’ll allow pairs to review their own small changes at their discretion. However, our general rule is “if you wrote it, you shouldn’t review it.”

The biggest difference from Wiegers’ description is that we don’t have formal meetings at this end of the spectrum. Our engineering teams at IBM Watson Pittsburgh didn’t find them to be efficient. Rather, code reviews are conducted at the reviewers’ leisure. Personally, I like to do them in the morning, because it gets my brain back in “code mode” before I actually have to write any code for the day. We have a cross-squad chat channel for asking folks from other squads to give their 2¢, too.

What problems does code review solve?

The primary purpose of code reviews is to reduce defects by carefully inspecting the submitted code for problems. The goal of the reviewer is to call out identified problems so that the submitter, or anyone else, can make additional changes and resubmit for subsequent review.

Aside from the primary goal of reducing defects, code review solves two major human problems.

Code review solves mental model synchronization. Say there are five people on a team. Do they all have the same mental model of the architecture and implementation?

Five people

Probably not, but hopefully they have similar understandings and concepts.

Five people with four different concepts above their head in the form of geometric shapes. Two have circles above them. One has a square. One has a square with rounded edges. One has a five-pointed star.

The process reduces the feedback cycle so that people can keep their mental model in synchronization with the architecture, even as it changes. With that team-wide understanding in mind, they’ll be best equipped to mindfully implement the architecture and provide feedback on its design.

The five people ordered by their concept shapes. The two people with a circle have the words 'on target' underneath them. The person with the rounded square has the words 'close enough' underneath them. The people with a square and a star have the words 'need guidance' underneath them.

What if someone has a different idea entirely, one that is so worth exploring that it could change the architecture? This has happened more than once on my team, where discussions on individual lines of code turned into new issues, new pull requests, and architectural changes.

Five people. The two people with a circle have the words 'open to new ideas' underneath them. The person with the square and rounded square have the words 'alternative solutions' underneath them. The person with a star has the words 'new idea' underneath them.

Secondly, code review solves tribal knowledge development.Tribal knowledge is the sum of stories of what was tried and failed, how best to handle certain problems commonly faced, and How Things Came to Be This Way.

A part of this is something called architecture oral history.

Architecture oral history requires that the team is both willing and able to retell the stories and keep the oral history alive. —Michael Keeling, “Creating an Architecture Oral History,” SATURN 2012

Architecture oral history collapses without a team to keep the culture alive.” —Keeling, a friend and fellow IBMer via Vivísimo

Code Reviews, or, more specifically, code reviews systems, enable a major part of that oral history to be written down, serving as a reference point—or even a starting point—for documenting the change of the architecture and implementation over time.

The most useful review systems will make comments searchable for posterity, and encourage reviewers to extract actionable items from comments or work tracking systems for future consideration.

Summary

Code review doesn’t just help build knowledge of application code anymore. It enables developers to continuously review changes to the infrastructure, and have an audit trail with logic and discussion around the changes. It enables architects, then, to continuously review changes to the architecture, and have an audit trail with logic and discussion around those changes, too.

Developers are included in this conversation, too, because the architecture documents should live in the same repository as the code it describes. This builds knowledge of how things changed over time and came to be this way.

In short, code review forces us to write down our discussions and make them searchable.


This series of blog posts is based on my presentation, Code Review is an Architectural Necessity. It was first shared at SATURN 2016 and subsequently at Github Universe 2016. This content is licensed CC BY-NC-SA 4.0.

Add Comment
One Comment

Leave a Reply

Your email address will not be published.Required fields are marked *


Karl Wiegers

The term “peer review” means “review of something by one’s peers”, “not review of someone by one’s peers”. The term has been well-established in the scientific community for generations. Publications are peer-reviewed by others in the scientific community before acceptance, thereby giving the work more rigor and the results more credence. (I went through this myself when I was a research chemist.)

The term was popularized in the software world by the Capability Maturity Model in the early 1990s, which established a Peer Reviews key process area. This was a generalization of the term “software inspection”, which is the most rigorous form of peer review and, ironically, was developed at IBM in the mid-1970s.

Calling all such activities “code reviews” is fine if you’re only examining code. But it’s a very good idea to review other software deliverables, including requirements of all sorts, designs, tests, project plans, and the like. So calling them all code reviews wouldn’t make sense. “Expressing” a design or architecture in code is not the same thing as examining the conceptual design knowledge that leads to code — it’s just code, and defects in the architecture won’t be detected until you have created the code. And requirements are certainly not “code” when they are represented in natural language.

There are other alternative terms, like “technical review.” “Peer review” is nicely encompassing of both the variety of deliverables that could be examined and the range of review techniques that can be used.

Reply
More Compute Services Stories

Ransomware’s Protection Winner, Zerto, on IBM Cloud for VMware Solutions

Zerto, IBM Cloud for VMware Solutions’ Disaster Recovery solution, has won the prestigious award of being the Ransomware Protection Company of Year at the 2017 Storage Awards!

Continue reading

Are You A Cloud Foundry User?

IBM actively engages developers, architects, and engineers in the open source community through foundations and initiatives. A Platinum Member of the Cloud Foundry Foundation, IBM regularly sponsors the annual Cloud Foundry Summit, where this year’s keynote speaker list includes Julian Friedman, Product Manager & Software Engineer. Developers use Cloud Foundry across every stage of the […]

Continue reading

Common Bluemix ID and Billing Questions: Part 2

You expect and deserve answers to questions as quickly as possible so that you can move forward with your business. In Bluemix Support, we receive a number of similar questions involving account changes, billing, and login issues. As we see patterns, we update our externally published FAQs to help you address questions without needing to open a ticket. However, if you need to open a ticket, we will address it as quickly as possible based on the documented severity levels in our Getting customer support information. In conjunction with my April 2016 Common Bluemix ID and billing questions article, here are some questions and answers:

Continue reading