Re:Stop Nitpicking in Code Reviews
by Bananaman28 on Wednesday February 24, 2021 @01:42AM
(#26247290)
I do this at work, even with newer developers, although I try give them more slack and time to adapt to our commit & PR style. Do I need to pick on minor formatting issues or a spelling mistake? Do I want to? No. My concern is that the long term cost of maintaining and fixing this when it was actually cought in PR.
So whenever someone nitpicks on my code it's usually for the better, i.e changing a method name.
When I started off as a code reviewer, I would frequently comment when "I would have done it differently". Now I make sure I only comment when "I would have done it better" and I always explain why it is better. Writing that explanation forces me to ask myself, "is this better?" and I also really want it to be substantially better.
There are a category of changes I request which I think of as "nits". Usually these are cosmetic changes that I think genuinely improve the code quality. I think they are worth asking why should we have a misspelling in a documentation string when we could not have it? It's just going to annoy everyone who reads that code until some unrelated change decides to fix it. When I have these kinds of comments, I preface them with "nit:".