The Code Review Pyramid

At my previous job, we had enforced automatic code formatting in pull requests, but most of the code was not (yet) formatted, and people were afraid to bulk format the code. I wrote a script that picked a few files periodically, formatted them, and auto-assigned a review. They hated it. Eventually I turned it off. I wasn't even pushing the style, by the way, just working with the most "legacy" code.

Before that, I worked on a team whose TL offered to hand-reformat my submissions, because clang-format didn't get his personal style right.

At my current job, we're a team of two and we know what we're doing, but there are some nits that have lead to lengthy, albeit adequately self aware, discussions (mostly C++):

- Is it ok to use assert()?

- Is it ok to use std::string literals? ("hello there"s)

- Should errors reported by an API have a corresponding error message? Should the error message be contextual?

- Should an API report fine-grained errors or just a component-wide failure value?

- Is it ok to add more logging? Is it just noise?

- Is it ok to report errors using exceptions?

- Are stringstreams too slow?

- Is it ok to write a strict parser for a specified format?

- Does a parser need a corresponding fuzzer?

- Is it ok to use macros for conditionally compiled multi-level logging?

- Should functions have contracts documented in source comments?

- Are regular expressions overkill?

- Should we use /bin/bash or /bin/sh?

- Is it ok to use exec in a shell script?

- Single quotes or double quotes?

- Should the release process be automated?

- How often should releases occur?

- Are variant types ok?

- Is RAII a useful technique or is it over-complicating code-hiding?

- Is this "simple code"?

... The list goes on and on. Can you guess, based on this list, what it is we work on? No, you cannot.

A team could vote on a Coding Style Guide to decide many of these things, but does anybody follow those? If you're going to give someone a hard time, you give them a hard time. </rant>

If you are having intense discussions about code style, that’s an organizational redflag and a sign there’s a talent issue in the org (it’s really bike shedding).

Code reviews become a hassle as soon as it becomes a career progression step.