|
Yes. Some people get personally attached to code. It’s incredibly frustrating. Some people use reviews to push dogmatic approaches to architecture and/or exert some kind of control over things. Whenever I meet these people in a code review, and they make unnecessary suggestions or whatever, my favorite phrase to say is, “I can get behind that, but I don’t think it’s worth the time to do that right now,” or, “I disagree, can you give an argument grounded in computer science.” With the latter only being used twice in my career, when someone left a shitload of comments suggesting variable name changes, and then again, when someone suggested rewriting something that was O(n) to O(n^2) and claimed it was better and wouldn’t give up. You want to get the team to a point where you can disagree and commit, no code will ever be perfect and there is no reason spending 3-4 rounds of change requests trying. I think the worst code review I ever had, ended with me saying, “if you’re going to be this nitpicky, why don’t you take the ticket?” (It was extremely complex and hard to read — and there wasn’t any getting around it, lots of math, bit shifting, and other shenanigans. The reviewer kept making suggestions that would result in bugs, and then make more suggestions…) He came back the next day and approved my PR once he understood the problem I was trying to solve. Even these days, where I work on a close team IRL, I’ve been known to say, “if there are no objections, I’m merging this unreviewed code.” And then I usually get a thumbs up from the team, or they say something like “oh, I wanted to take a look at that. Give me a few mins I got sidetracked!” And I’ve even heard, “I already reviewed it, I just forgot to push approve!” Communication is key in a team. Often, if the team is taking a long time to review, give them the benefit of the doubt, but don’t let yourself get blocked by a review. |
Anything else is un-needed grooming that's more about the other developer's ego, not about good code (sometimes its to follow some other constraint, but its a good sign the person has a personality issue).