Code Review at Criteo

Taking Care of our Engineering Culture… One Toilet at a Time!

Introduction to Code Review

When I first joined Criteo, code review was still a new thing in the culture. Taking a page from industry best practices, the DevTools team (the guys whose mission it is to make our lives easier) had installed Gerrit, an open source code review tool. They did a small “initiation to code review” presentation at an R&D All Hands (our monthly engineering gathering) and then set the tool running. From one day to the next, any c# code checked in to our repositories had to go through Gerrit and get a +2 from another developer.

R&D All-hands gathering

Mixed Reception

The reception was mixed — some teams (we had about 20 total at the time) loved it and immediately took to the new tool. For other teams, it was a process burden that didn’t really seem to add any value.

Worse yet, when engineers were making commits between projects, code review became a huge source of friction. Every team had developed their own unwritten rules of code review, and the only way to discover them was through the review process itself:

• Some teams teams would post a few comments and put a score that blocked the review in limbo.
• Some teams would reject all code that didn’t match their quixotic style guide of the team.
• Some teams simply ignored incoming code reviews…. which then piled up until frustrated engineers flamed them or stomped over to get an explanation.

Here we were with this great tool that is supposed to improve the lives of the developers and the health of the code-base… .and yet it just wasn’t working out as we’d hoped.

Why do Code Review? 

I noticed this frustration and quickly found a couple other engineers who felt the same way. Together, we had a lot of long talks about how *good* code review can be, eventually coming up with this list:

• Ensure a high standard of code quality and consistency across our code-base
• Spread knowledge and distribute ownership of our code base
• Facilitate an environment of continuous learning and growth for our engineers
• Catch bugs early on before they get into production

Code review in session.

The Code Review Champions

We shopped that list around a bit, talked to some DevLeads from different teams and eventually organized a group of people who all felt the same way. We called this group the “code review champions” and together we had a huge brainstorm on the Code Review process at Criteo. Why and where was it broken and how could we make it better?

After lots of debate and negotiation, we formalized our thoughts/suggestions in a “Code Review Manifesto

More code review in action. Photo: Sylvain C

The Code Review Manifesto

We took that manifesto around and got sign-off from all the senior engineers, DevLeads and engineering managers in the whole company. Next we made a presentation at the engineering all hands… a bit like the one that they’d done earlier, but this time with a bit more focus on the cultural aspects of code review, and with explicit buy-in from the DevLeads.

Even so, we knew that a presentation wasn’t going to be enough. What about newcomers after that? What about those who weren’t there for the all-hands? Limited by a 5 minute time slot, we hadn’t actually had enough time to get to all the material in the manifesto. Anyways, hearing something one time is often not enough to stick and actually change someone’s behavior/habits.

Take it to the Toilets

Take it to the toilet

And so it was that we had the idea of taking code review to the toilets. Inspired by google’s toilet series Testing On The Toilet, we got some plastic sleeves, some yellow-tack, and some terrible graphic layout skills.

We created Code Review On The ToilettE, or CROTTE for short. (for the non-french speakers)

The process of translating the ideas of the manifesto into a a4-sized message was a good challenge, and every month for a year we got different collaborators from throughout the r&d department to step up and give it a try.

The topics we covered are:

• Optional Comments
• Negative Reviews are Normal
• Small Reviews
• 24h rule
• Giving Context
• The Power of Reverting
• For Another Review
• Review All at Once
• Address All Comments

After a full year of publication, we figured that people were probably a bit tired of reading about code review during that sacred part of the day, so we took a pause.

We sent out a survey to get some feedback on peoples’ reactions to the initiative, and they were quite positive:

• 70% of respondents said they learned a few things or more
• 99% of respondents liked the format 

Changing Culture is Hard

One of the things we’re really proud of here at Criteo is our engineering culture. As you can see, it’s something that everyone here participates in (even someone who just joined the company a month ago). Keeping this culture cool and united in the midst of rapid growth (150 to close to 400 engineers in two years) is not easy. Sometimes, like with the code review initiative, it takes a bit of effort. But it’s an effort that is supported and encouraged here at Criteo.

In fact, at any given time, we might have several of similar efforts going on in the R&D (e.g. code style, security, documentation). These are all bottom-up efforts pushed by Criteo engineers who care enough to invest their time and energy to make things better.

Post written by: