4 ms·

This happens in PRs pretty often. Sometimes I meet reviewers who are very nitpicky. It takes like 5-6 iterations before they approve. I was one of them. A juni

by nojvek 6y ago

This happens in PRs pretty often. Sometimes I meet reviewers who are very nitpicky. It takes like 5-6 iterations before they approve. I was one of them. A junior engineer on team mentioned that I made them feel they weren’t competent. I didn’t realize how every comment I made tore their morale. It was a defining moment for me to approach every PR with understanding that even a couple of lines of code has hours of work of a human with feelings and ambitions behind it. I try to praise good work when I see it, stay away from NITs and put all my feedback in single review within ~4hrs. If they address feedback, then it’s approved. I avoid multi iterations if I can. If there are bigger concerns then I’d meet them in person to explain the why and philosophy behind my suggestions. Most people really appreciate “critique in private, praise in public” methodology. Also linters and formatters are great. If a thing is NIT-ed a couple of times, create a rule for it (auto fixable rule is even better). I wrote so many custom eslint rules at Mixpanel. Coding is a lot more fun when the IDE fixes little NITs automatically on save.

dan-robertson 6y ago
Where I work, our code-review system allows anyone to push changes to any PR (provided they have permissions for the repo). It’s a great solution for people who have strong opinions on nit-picky things: they can just make trivial reasonable changes (eg rename a variable called x) instead of leaving a comment asking the author to make them. There’s obviously some middle ground like “I didn’t understand this function so I tried rewriting it: ... <new code> ... what do you think?” And it doesn’t work if you can’t have a good interaction with colleagues or if someone is too nitpicky. So it basically still works fine. One funny issue I had was when two reviews each attempted to make what appeared to be a trivial change only to have the code fail to compile. We added a comment the second time.

by nojvek 6y ago

dan-robertson 6y ago

Where I work, our code-review system allows anyone to push changes to any PR (provided they have permissions for the repo). It’s a great solution for people who have strong opinions on nit-picky things: they can just make trivial reasonable changes (eg rename a variable called x) instead of leaving a comment asking the author to make them. There’s obviously some middle ground like “I didn’t understand this function so I tried rewriting it: ... <new code> ... what do you think?” And it doesn’t work if you can’t have a good interaction with colleagues or if someone is too nitpicky. So it basically still works fine. One funny issue I had was when two reviews each attempted to make what appeared to be a trivial change only to have the code fail to compile. We added a comment the second time.