Hacker News new | ask | show | jobs
by kortilla 2284 days ago
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”.

1 comments

> 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”.

I think you're describing style errors, not grammar errors. For example, the Google style guide says that (certain) comments should be complete sentences. Asking people to use good grammar (and capitalize/punctuate) those sentences as though they were documentation (because they are!) is valuable for reviewers.

I say this as someone who, when reading unfamiliar code, is able to understand it more readily if the inline documentation is easy to read. "tcp" vs "TCP" doesn't impact that, but other grammatical nits absolutely due.

> I think you're describing style errors, not grammar errors.

The entire thread (not started by me) is about nit-picking punctuation and now you’re trying to explain to me that I’m describing style errors. So what? It’s useless feedback regardless of which chapter in the Holy Google Docs covers it.

> Asking people to use good grammar (and capitalize/punctuate) those sentences as though they were documentation (because they are!) is valuable for reviewers.

Right, it’s valuable for the reviewers because it makes it seem like they provided feedback when they have nothing of substance to add. It’s not valuable for the author though nor is it realistically valuable to future people reading the code in question.

>I say this as someone who, when reading unfamiliar code, is able to understand it more readily if the inline documentation is easy to read.

I think maybe you don’t know what “nitpick” means. https://www.dictionary.com/browse/nitpick

If it has a meaningful impact on the ability to read it, it’s not a nitpick.

My experience differs. Consistent and well formed prose in docstrings and comments absolutely helps reason about unfamiliar code. Nitpicking for good formatting, spelling, and grammar is valuable in code comments just the same as in documentation or any other form of technical writing.

If the comments aren't important enough to spend care and thought on, they should be deleted.

> If it has a meaningful impact on the ability to read it, it’s not a nitpick.

This is boring semantics. Given what you describe as nitpicks, we draw the line of meaningful impact on readability in different places. So I'm claiming that some of the things you describe as nitpicks in fact aren't.