3 ms·
Properly communicating when a requested change is optional is actually one of the most important code review skills you can have. By default, developers assume
by jlund-molfese 3y ago
Properly communicating when a requested change is optional is actually one of the most important code review skills you can have.
By default, developers assume that a requested change is mandatory. So as a reviewer, it's up to you to explain the difference between a change you're suggesting because it's more concise to use a list comprehension instead of a loop vs a change you're requiring because the change list introduces a race condition that will cause customers to lose money.
If you do that, you'll make everyone you work with happier
- rsyring 3y agoWe prefix any comment or request with "nit", as in "nitpick" when the reviewer knows they are being picky but it's still something worth mentioning. The coder in that case knows they can ignore that without consequence if their preference or perspective is different. The reviewer also knows that and doesn't feel bad if that advice is ignored. It's been helpful for me because I'm a picky reviewer and feel compelled to mention things that my years of experience tell me are "wrong." But I've also come to realize it's often not wrong, just not as good for potentially non-obvious reasons. But my devs will never learn the potentially "better" way if I don't point it out. So "nit" gives me a way to communicate and teach without the pressure of the dev needing to make an immediate change.
- jlund-molfese 3y agoSame! I also, if I notice I'm leaving a lot of constructive comments, deliberately try to leave some positives ones too. Like "Nice! Glad we're adding typing here". Just as important to promote positive habits as discouraging negative ones.