October 19, 2016 | Written by: Colin Dean
Categorized: Community | Compute Services
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.
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.
My concept of code review, and my team’s practice of code review, falls somewhere between “team review” and “pair programming.”
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?
Probably not, but hopefully they have similar understandings and concepts.
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.
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.
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.
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.