Notably, a good code review should also bring up the good parts of the submission, and not just concentrate on the errors. Not only does it make the recipient feel better to get positive feedback among the negative, but it helps them learn about good practices too. Just concentrating on the errors doesn’t really tell them which things they’re doing well.
Many reviewers concentrate on just finding mistakes, and while it’s useful it’s sort of the bare minimum; a good code review should be educational. Especially if the submitter’s a more junior coder, in which case it’d also be a good idea to not just outright tell them how you’d fix some problem, but sort of lead them to a solution by asking them questions and pointing things out and letting them do the thinking themselves. But still, experienced coders will also benefit from well-structured feedback, it’s not like we’re “finished” and stopped learning.
Yes, I tend to do that, and thankfully some of my colleagues do too. Clever but readable solutions, following good and relevant practices, clear documentation, making a good MR description that makes it easier to review, and more.
That’s great to hear. It’s thankfully becoming more common in general, and we can all do our part in spreading these practices.
I tended to actively evangelize for it when I was managing coders or teams. Unfortunately it’s still not all that uncommon for coders to be downright offensive when giving feedback, like not necessrily quite Linus-level rants but things like “this is idiotic, this is stupid, that’s shit, why would you do that” etc etc. The usual explanation I’ve gotten is that they’re just being “honest” and saying what they think, and it’s not their problem if the reviewee (is that even a word‽ I can’t English today) gets offended. Some even get all huffy about it, like “oh we’re just supposed to coddle them and never say anything negative so their little feefees don’t get hurt?” And I mean, yeah, getting honest feedback definitely a good way to learn, but it’s not like the only way to point out errors or problems is to be a cunt about it.
