[yrgulation]

Too many think code reviews are an opportunity for endless debates over personal preference. A code review should be fast and cover blatant good practice violations and architectural mistakes. Everything else should be taken care of by linters and tools. If a reviewer wants code done in a different way they can write the code themselves.

[98codes]

One thing I've enjoyed where I am now is that PR comments come in two flavors. The first, actual feedback. The second, borderline pedantic issues that are prefaced with "nit: " in the comment. Nit comments are safely ignored but are there so that if the author wants to put in that change while changing some other issue, then OK.

[gknoy]

I especially like Github's new option to make a _suggested diff_ of what you want changed. Typo fixes, comments re-worded, etc. It really reduces friction, both as the person making the suggestion, and as the person who authored the PR.

[agentwiggles]

I completely agree, suggesting the actual diff is the ultimate "put up or shut up". One of the most annoying review situations is when you get some vague comment that doesn't spell out what change is being asked for.

Although the best response to that sort of thing is just a quick DM - "hey, I'm not sure how to interpret this comment, wanna hop on a call for a minute and explain?"

Again - put up or shut up. Want a change? Cool, let's pair on it. I want the code to be good too. But I'm not going to just read your comment and based on the vibe I'm feeling shoot off some change, only to find out that it wasn't what you meant.

[jeffbee]

Yes, hopefully someday GitHub will have all the major features that Gerrit has had for years. By the time I'm ready to retire, GitHub PR review UI should be up to approximately 2010 standards.

[wahnfrieden]

the reviewer should make 'nit' changes directly rather than pass a mini-ticket for evaluation back to the author. why are reviewers so scared to modify prs at orgs?

[atq2119]

That's an interesting thought. Personally, I feel messing with another person's personal branch is dangerous. They might already have some change locally, which is going to lead to surprises, and most of the people I work with unfortunately don't know Git well enough.

It's something that could be addressed on an organizational level perhaps.

The other point to consider is the whole teach a man to fish vs give them a fish thing. Teaching is better in the long run.

[wahnfrieden]

Teaching: you can still comment and edit. And your edit in the PR is reviewable by the author.

Org level: absolutely. Solvable thru tooling.

[alecbz]

I agree that anything that _could_ be covered by an automated check ought to be. But don't think I'm convinced that everything else is either an egregious mistake or isn't worth discussing.

IMO a big part of what you ought to be reviewing for is readability, which does sometimes overlap with personal preference. But there's a spectrum from "I'd indent these columns a little differently" to "it's hard for me to follow what's happening, I think it'd be clearer if we organized things like...".

[yrgulation]

"I think it'd be clearer if we organized things like..." any such guidelines should be agreed upon beforehand, otherwise what's the expectation? That people rewrite their code to accommodate someone's needs? Reading, other than what's in whatever coding style the team uses, is subjective. Best to agree upon what the whole team prefers beforehand.

[Jensson]

The expectation is that you talk with the person who did the review and discuss the best place to put things. Those discussions are good for code health, and it is much easier to have them when you have new code in front of people so they can see what it looks like.

If you can't have those discussions in a code review then your code review process is too slow, and you ought to fix it so that it is easy to have these discussions there.

[alecbz]

Reading is absolutely subjective, but that doesn't mean it can't or shouldn't be optimized for (just like with regular written language -- editors are a thing).

Being a good code reviewer (just like being a good editor) is absolutely its own skill though, and people can be bad at it.