Hacker News Reader

Comment by tunesmith | Hacker News Reader

Getting reviews from multiple people that disagree on style is definitely an org problem that sucks to be in the middle of.

However, I'd say that getting nit comments on code that surrounds your changes, but that you didn't change, is still fair game. It's part of the leave your campsite cleaner than you found it. It depends on the culture though - if it's suggested in the manner of "since you're here, here's an opportunity for how to improve this area of the code", that's better then acting like you made a mistake in failing to change it.

View on HN

This can add cost and latency to the change author and distracts them from the project at hand. And because it costs the reviewer basically nothing to say, "While you are in there....", it slows the original author's work.

Even worse, if there are enough of these minor changes, some other reviewer may take issue with their "small, focused changes" preference, and ask you to split it.

This is why Google has the "It doesn't have to be perfect; it just needs to be better" standard: So reviewers can't impose undue costs on authors.

I try to avoid nits totally unrelated to the changes at hand, since on a subconscious level they may discourage people from even wanting to touch older/less loved files at all.

The critical exception being avoiding issues due to path dependence.

E.g while a change is "correct" is doing X poorly because of surrounding issue Y. So we should fix Y now instead of building atop it.

I fought with the review admins about this crap a lot when I was at Google. When you're working toward readability, code that isn't related to change is not in scope. The reviewer can comment on it but you're not expected to change it. That is in the official "rules".

Something I find helps with this in particular is only allowing style comments with citations to an actual style guide item. (I talk about domain-specific style guides as "crystallized arguments" - we agreed on this and wrote it down, not because it's necessarily right (though it probably is) but that we really wanted to stop wasting time arguing about these particular things.)

Reply on news.ycombinator.com