Hacker News new | ask | show | jobs
by sanderjd 3104 days ago
Consistent style is useful because it reduces cognitive load when reading unfamiliar code, but everyone should have automated tools that do that "95%", and most people don't. This is something that Go got very right by building it straight into the compiler but automatic linter-formatters exist for most languages, and it's useful for someone high up in the early technical team to just pick some flags they like for one of those and integrate it into the review process.

Once you eliminate that 95% overhead (which I think is an exaggeration, but I'll allow it can be high), the value proposition of code review is an absolute no-brainer.

1 comments

linters and auto formatters add a lot of value, but some style changes don't fall into that. e.g.: naming things, or just when the language provides more than one way to do something, which one you choose.
I don't consider either of those to be matters of "style". Naming is more akin to documentation, and as such, it is a very useful thing to review: if names don't make sense to a reviewer now, they're even less likely to make sense to someone reading them in the future.

Picking which among a few ways of doing something is idiom, not rote style. I agree that this is one of the least valuable parts of review, but people ramp up quickly on it in my experience, so it isn't constantly coming up. If an idiom actually matters, for instance if it is protecting against some commonly problematic pattern, it may be possible for tooling to catch that too. If it really doesn't matter which way its done, the reviewer should just let the author do it their way, and if they don't, the author should just change it without making a fuss. But yeah, arguing over idioms can be a problem. It helps to write down decisions the team has made in the past, so that you can just point dissenters to the documentation.