Hacker News new | ask | show | jobs
by alecbz 1304 days ago
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...".

1 comments

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

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.