Hacker News new | ask | show | jobs
by philipnee 54 days ago
Just want to comment here. I hate reviewer leave a bunch of nits and stamp the PR. This is ambiguous, are these nits asks or just you opinions? What if I dont address all of them. Also folks need to take rejection lightly - your reviewer wants you to address something, thats really it.
2 comments

Hi, I do this.

> are these nits asks or just you opinions?

If I've approved the PR, then these are changes I'm asking you to do, but not ordering you to do. You are free to say "no" to my request

> What if I dont address all of them

Then you will have decided that you don't agree with my recommendation and that's OK.

I only ever do this with people I trust - I am trusting you to review each of my nitpicks and make an informed decision if they're necessary or not. Generally I'd like you to reply to the ones you don't do with a reason though.

On Github, there is a way to leave individual comments in the code and in addition give a review a summary.

In addition to hitting the "approve" button, I typically spell it out explicitly in this summary: "Please check my comments and see if anything makes sense to implement."

Often, I also take this opportunity to point out the "one" most valuable change in my opinion.

If the developer of the code doesn't find any of the comments to be applicable/usefuly, they can always go ahead and merge it right away.

I try to tag the line-by-line comments with little labels like [Unimportant] or [Style] so that someone going through them has an idea of their (un)importance without reading the whole thing.