> When it comes to code reviews, it’s a common phenomenon that there is much focus and long-winded discussions around mundane aspects like code formatting and style
Does this really happen, or has it just became a meme? I haven’t worked at any company where we did not have auto formatting set up with a common style config, along with some form of linting.
It really happens, though hopefully less and less.
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>
Code style is a trivial issue to solve with modern tools. And even before that, most review I’ve seen simply left a single comment “Hey, you forgot to indent ;)”
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).
Does this really happen, or has it just became a meme? I haven’t worked at any company where we did not have auto formatting set up with a common style config, along with some form of linting.