[kortilla]

Nit-picky reviews are a smell, not a good thing. It can often mean that the reviewer is not really reviewing the big picture (is the intent of the change being fulfilled, is this the right place in the architecture, etc) and it making up for it by focusing on irrelevant details.

Seriously, code review on punctuation in a comment is not good in any scenario and it speaks nothing to the legibility of the code itself. If anything, it might mean the code is unreadable and the reviewer is too embarrassed to point out that they can’t follow it.

[joshuamorton]

This conclusion doesn't, at all, follow from the premise.

While yes, nit-picky review could mean the reviewer doesn't understand the big picture, an LGTM with no comments at all is more likely to be indicative of missing something.

What you really mean is that nit-picky reviews that don't pick up on actual logic or functionality issues are a smell. Which is true, but isn't usually the case at Google[0]. The chapter on teamwork covers some relevant concepts (like psychological safety).

[0]: https://testing.googleblog.com/2018/05/code-health-understan...

[kortilla]

The comment I replied to is literally highlighting people nit-picking punctuation as if it’s a good thing.

[joshuamorton]

Which it is!

You seem to be assuming, without prompt, that nit-picking punctuation comes at the expense of a thorough overall review.

Code review on punctuation is absolutely an important part of readability. Consistency makes understanding and scanning (for human readability) and potentially parsing and modifying (for machine readability) much easier.

As someone who both reviews a lot of code and reads a lot of code, consistent documentation with good grammar and punctuation is enormously helpful.

[kortilla]

I’m not assuming, this is based on my experience at Google. People that focused on punctuation rarely provided anything more than the superficial review.

[joshuamorton]

Again you're begging the question.

People who focus on punctuation do so at the expense of everything else, sure. But people who give thorough reviews give thorough reviews, including nitpicky comments.

I've given and received thousands of reviews at Google (in multiple languages, across multiple teams), and can say that my experience has been fairly consistent: code review is high quality and detail oriented, including, but not at all limited to, small things like grammar and comment formatting.

[kortilla]

I’m not begging the question, I’m just sharing my experience from working at Google and other companies. By a wide margin, the most useless reviews were ones nit-picking formatting, punctuation, etc.

> But people who give thorough reviews give thorough reviews

Ok.

> including nitpicky comments.

Disagree. Pointing out that someone forgot to capitalize an initialism in a comment is not thorough, it’s a time suck. A thorough and competent reviewer will realize it doesn’t matter and will know not to distract the CL owner will such irrelevant trash.

Sure, there is an a small section of the Venn diagram that leaves good review comments and sprinkles in “nit:” turds, but that group is much smaller than both the fully incompetent nit-pick group and the competent reviewer group.

Seriously, unless a code comment is illegible or it’s being compiled into user-facing docs, it’s absolutely useless to leave something like “nit: it’s TCP, not tcp”.

[jbn]

what about nitpicks related to whitespace? These are an endless source of time-wasting, IMHO.

[joshuamorton]

At Google or in general?

The closest I've seen to this is comments of the form "can you reformat this" or "this doesn't match the rest of the file". Google's style guides have fairly strong guidance on formatting, so there's usually a "right" way (enforced by a linter), and if not, then usually you defer to the existing style in the module.

[jbn]

I meant in general. Except python-like languages that make it a non-issue, in C/C++/Java I think people attach way too much importance to spacing, trailing whitespace, tab vs spaces, and column width.

I would agree with linting or just deferring to existing style, but in truth I care little about this, it's just not significant to me (yet others seem to overvalue it, to the point of talking exclusively about this).