Should you call out code style issues in a code review?
One of my friends works at a software company where all code changes must be reviewed before they’re deployed to production – I think that’s great. However, he finds that his team spends too much time discussing code style and formatting issues in their code reviews.
He feels that code style and formatting discussions slow down development because they often require a lot of back and forth between reviewer and reviewee.
And sometimes these conversations take a turn for the worse and erupt into heated arguments.
I wanted to take a few minutes to share my thoughts on this topic.
Is pointing out mechanical issues like code formatting in a code review a good idea? Is it worth your time?
Giving feedback on things like code formatting or adherence to a style guide can turn into a never-ending story and cost a development team a lot of time.
In the worst case this kind of feedback can soak up huge amounts of engineering resources:
-
For example, if review comments need to address style issues, functional problems with the code (== actual bugs!) might not be identified and get waved through.
-
It costs valuable developer time to argue over code styling and formatting. This might also lead to conflicts within the team by turning into a holy war around minutiae like “tabs vs spaces”.
However, code formatting issues should almost never come up during a code review.
Mechanical style issues, like tabs vs spaces, or basic naming rules for variables, shouldn’t even make it to the code review stage.
Automated tools, like linters and code formatters, should identify these issues before the code is opened up for peer review. Run these tools on a build server so that devs don’t need to trigger them manually.
Remember that the best code reviews should provoke discussions — and spending time on basic formatting issues won’t help with that goal.