Code reviews are cultural

February 10, 2016

Only a few years ago, I was a recent addition at Workiva, so the difficulties that new recruits experience are still relatively fresh in my mind. I've spent a great deal of time reflecting on my own challenges upon joining this team and have been fortunate enough to help others on this same journey.

Some observations I have made coalesced into some epiphanies about the software engineering culture at Workiva, and I’d like to share them with you.

1. Code isn't the place for Baskin-RobbinsTM

Let me describe one facet of our software engineering culture to help you understand what I mean. The software engineering process at Workiva involves a phase where peers review code prior to it moving forward toward being used in production. (If your process doesn't involve this phase or you're not sure what I'm talking about, I urge to do some background reading on code reviews before continuing to read this post.)

Prior to life at Workiva, I was accustomed to a less structured process where success as an engineer solely hinged on a feature being ready on time and within budget. In other words, the code was not viewed as a part of the end product. There was little emphasis on ensuring good quality code, though that was the intention of each code author. The problem was that this often proved to not be enough.

Why aren't good intentions enough?

Without a code review phase, there are several different contexts as each engineer has his or her own definition of high-quality code. Without a code review phase, a team of engineers are unlikely to come to a consensus definition for high-quality code, which can result in an fractured and inconsistent codebase.

Common symptoms of a fractured codebase:

Inconsistencies

  • Conventions are different in portions of the same codebase
  • Utility methods are used in some places, but not others
  • Testing methodologies (no tests, % coverage, unit vs. integration vs. functional test usage)
  • Varying level of/style of documentation
  • Differences in code formatting (whitespace usage, which line brackets fall on, line break length, etc.)

Conventions

  • Naming conventions for variables/methods/classes
  • Methods that perform in place data manipulations vs. returning new result objects
  • Different methods for passing parameters into methods (use of optional parameters or not/use of list of explicit parameters vs. a single parameters objects, etc.)

How does Workiva prevent fractured codebases?

Code reviews are one way that we can ensure that everyone contributes to a common code standard and more importantly, that everyone agrees on what that standard should be. We also use tooling to enforce some of those conventions (code linters with agreed upon settings/configurations—automatically verified as part of the PR/CI process). This results in one flavor of code and provides a number of benefits to us engineers.

How do we benefit from having our code formally reviewed?

  • We will be more confident in our changes because we trust that our peers will help us weed out any typical human errors like spelling mistakes, deviation from naming or format conventions, etc.
  • We will be become better at accepting, leveraging, and providing criticism. This means that we will be able to increase our software engineering skill level at a much faster rate than without code reviews.
  • We will play an instrumental role in establishing, maintaining, and enforcing code standards in our organization.
  • We will spread knowledge of the entire code base amongst the team.
  • We will become better collaborators in major software engineering/architecture decisions.

This sounds too good to be true.

Remember earlier when I said, "Good intentions aren't enough?" It turns out that code reviews by themselves aren't enough either. We need a way to extract the maximum value from a code review for everyone involved.

This brings me to another epiphany.

2. Code is courage.

I remember like it was yesterday. One of the first changes I ever made at Workiva took me days to figure out. Once I had it working, I polished up my code a bit and submitted it for code review. I was feeling supremely confident because not only did my code work properly, it made sense if one read it.

When my code reviewers read it, they informed me that it actually didn’t make sense to them. That was a good dose of humility.

Am I an imposter?

At first, I wondered how these other smart engineers could be so wrong in their comments. It was at that moment that imposter syndrome began to creep in.

It didn't take me long to realize that the comments were valid, and I needed to make the changes suggested as well as reevaluate my whole belief structure around code quality.

What I discovered after speaking with my mentors was that the software engineering culture at Workiva values a relentless pursuit of the highest possible standards. Good enough just isn't good enough. Broken windows are categorically despised.

We’re at our most vulnerable as engineers when our code is being reviewed.

When we submit code for review, we're making ourselves vulnerable to hearing things that we may not like. For software engineers, code is the one verifiable artifact that we leave behind. Our code tells others a story about what our intentions are, where our skills reside, and how a system should work. Sometimes, it also speaks to which constraints we're under.

It takes a ton of courage to put code out there and invite a critical review from our peers, or worse, from people we admire. We're at our most vulnerable as engineers when our code is being reviewed.

We're also on the precipice of greatness: our greatness as engineers is directly tied to our ability to grow. One of the best opportunities for personal growth is a good critical code review.

It may take just as much courage to call out your peers for deviating from the norm in their code reviews. If you ever think about sparing your peers’ feelings by going light on them in a code review, stop. You're not doing them a favor, you're actually stunting their growth. By raising the bar, we foster their growth.

One other common courage-centric scenario is when we don't understand something on a code review. It's great when we ask questions on code reviews because it often spurs discussion and knowledge sharing. Chances are, if you don't understand something, there are others who also don't understand. Putting your questions down on a code review ensures that not only are they answered, but they’re also documented.

You might already have predicted what's coming next. Courage just isn't enough.

3. Culture is king.

This might be one of those cases where one has to be in a culture with the right mix of professional freedom, competent peers, and thoughtful leaders to fully appreciate that the right professional environment will make us better. Admittedly, after the first few months after joining Workiva, I was questioning how well I fit in with all these intelligent and high-functioning professionals.

Sure, I was self-motivated and loved learning new things and actually applying new knowledge, but there's nothing more motivating than feeling like your success directly impacts the success of others.

My first few attempts at producing code were rough. There were days I wasn't sure how many rounds of code review it was going to take before my code would be ready. But here's my realization: it's okay to need a few extra rounds to figure out what the expectations are.

Looking back at those first few weeks, I've come to appreciate just how much progress I had to make. Thankfully, others have expressed how they too went through this phase of learning and growing. We've come to celebrate these types of struggles because it's an indication of legitimate growth.

This is the beauty of a collaboration culture—those with lesser skills will more quickly reach parity with their peers. It’s purely due to the constant reinforcement of standards and the realization that their true potential lies way beyond where they think it does.

There are probably many other epiphanies just waiting to be written down, but these are the ones that I've found most impactful to myself and my peers.

If you ever find yourself thinking, I want to grow as a software engineer and I want to be challenged, then reach out to us, and let’s build something amazing together.

Baskin-Robbins is a trademark of BR IP Holder LLC.© 2005-2013 BR IP Holder LLC. All rights reserved.