Ask HN: How would you handle pedantic code reviews?

https://news.ycombinator.com/item?id=31724183

8 4 4 years ago

news.ycombinator.com

I'm in a SW team where I find myself spending time and energy in dealing with pedantic no-value commends in my PRs. Most of them coming from a certain colleague who is clearly insecure and not as competent as the rest of the team. So she is trying to hide that under various forms of busytalking and commends like the above.

The triviality and non-value of her commends is not disputable as I am not the only person that sees that and despite the "rigor" she misses obvious bugs (I left some in in the latest PR just to make sure).

So all in all she's a no-value PITA. Despite that I've been kind and condescending so far but it's starting getting on my nerves as I have more urgent things to do than pacify or comply with such commends (e.g. BusVehicle should be named CoachAutomobile).

How do you deal with similar cases? What do you suggest?

efortis• 4 years ago

If they are superficial or stylistic I simply make the changes without thinking it twice.

Also, you can ask them for a patch with the suggestions (instead of dealing with a bunch of comments).

To me the difficult ones are the bad suggestions. In those cases, tell them why you disagree, and if they don't agree, ask them if they can commit their suggestions.

foxyv• 4 years ago

These used to bug me a LOT. But I quickly realized it takes me about 2 seconds to amend the PR and keep the team happy OR 10 hours to teach the person how to code like a "10x PRO!" I quickly realized that the real 10x pros just kinda go "Eh sure, why not?" make it work, and get the job done in record time.

Lately I spend a lot of time trying to get my ego out of my code. I let the new guys and gals spend time pissing in the soup. I just use whatever weird Lombok Module/Code Standard/Static Code Scanner/New Library they choose and make it work one way or another. Then I look super cool when I download the source code for their new slick library and add a feature to cover a use case they are stuck on.

sky-kedge0749• 4 years ago

You should have the freedom to reject trivial, opinionated PR comments with a simple "no thanks," no explanation needed. Your team should err on the site of supporting the PR as originally written. If you reject a ton of comments then the situation should resolve itself somehow, either she'll stop making them, or it will take you minimal time to move past them, or she'll complain to management and you can all discuss the situation together.

Don't take it personally, don't make it about her personally or about her feelings of security or her competency or whether she's a PITA, definitely don't intentionally leave test bugs in your code.

throwaway019254• 4 years ago

If it's something that takes me 10 seconds and doesn't make anything worse then I just update the code.

If it's something that will make stuff worse - I will explain why we should not do it.

If it's a comment about formatting the code, I would add a linter/prettier that will check/format the code automatically. If your colleague wants to have different rules, they can update the formatter config.

Ask HN: How would you handle pedantic code reviews?

Related Stories