Hacker News new | ask | show | jobs
by jjgreen 3391 days ago
Reject it.

I've done it a few times, there was a fuss, there was a discussion, it got sorted out.

At the same time I've had a couple rejected (due to not adopting the reviewer's preferred code style). Same thing happened.

2 comments

I agree. I would flat out say "I won't accept this change".
Agreed too; that's how it is at Google. Code reviews aren't "hey reviewer, take a look at this code so you understand it and rubber-stamp it".

As a reviewer you're the gatekeeper, and no code is submitted unless the author and all reviewers are happy. Happy as in, if you approved the change and the author leaves tomorrow, you're the maintainer of that code. No one should be rude in the review, but the understanding needs to be that the power lies on the reviewer to hold their approval indefinitely until they're happy with the code. That's a cost for the author and costs almost nothing to the reviewer, so as an author you learn very fast to not waste time discussing whether something "is not important enough": You apply all suggestions you don't disagree with, and explain your disagreement about those that really matter.

This is why it's helpful to have a consistent code style across the team, or at least across the codebase, so that personal opinions in either direction don't cause too much conflict in code review. Some companies value low quality but running code now over better quality code tomorrow.