What's your best PR etiquette tip?

What's your best PR etiquette tip? From the standpoint of reviewee or reviewer, your choice.

Discuss 👇

Comments

[Jean-Michel 🕵🏻‍♂️ Fayard]

As a reviewer, my goal during a PR is to ask questions, not to be a gatekeeper.
I approve the pull request once I've done that.
I trust the reviewee to find good enough answers, or to ask for help if needed.
I know I have succeeded if in a next PR my fellow programmer ask herself the same questions.

I think often about this article from @daedtech : How to Use a Code Review to Execute Someone’s Soul

[Spyros Argalias]

Nice. I think it's good to have trust that your colleagues are good. Good points in general.

[Jean-Michel 🕵🏻‍♂️ Fayard]

That's the point, pull-requests were invented for the open-source model where you have zero trust with the first time commiters. Instead of emulating a zero-trust environment, build trust!

[Waylon Walker]

not to be a gatekeeper.

Fantastic tip

[Spyros Argalias]

Be very polite. Remember that people are sensitive and getting a PR comment can make people very defensive.

This means to talk about the code, not about the person. E.g. "the code is doing X" rather than "you wrote X".

I also try to write less assertively. For example, I'll say: "I think scenario X is likely to happen. In that case, there might be X problem. What are your thoughts (or, if I'm very confident the issue will happen, I'll say "can we update the code to prevent this")?". I won't say "the code fails under scenario X. Please fix it".

Feels a bit like treading on eggshells, but I think that's for the best.

[Waylon Walker]

Feels a bit like treading on eggshells, but I think that's for the best.

for sure. I somehow found myself in the position of team lead with very little of my code going through review. Now I review lots of code, so its hard for me to find words for things.

I have a bunch of linters and checkers running so pedantic stuff is already taken care of. I have been known to be very hyper focused on robustness and reliability. I am very focused on the next person being able to read, understand and change the code. If you have set the next person up to fail by connecting two things that shouldn't be or not connecting two things that should be I'll call that out.

[Spyros Argalias]

Very nice. Yeah automating the little stuff makes everything better. Less time wasted on it and you can focus on the important things more.

The rest sounds like you're reviewing code as it's meant to be reviewed :)

[Waylon Walker]

this comment made my day!

[Suzanne Aitchison]

Conventional comments I think are hugely helpful - for example, instead of commenting on a PR with:

this could be extracted to a helper function

This would be more useful:

suggestion (not blocking): this could be extracted to a helper function

It's really hard to receive PR reviews with lots of "general comments" - there's a lot of mental processing and effort involved in establishing what needs responded to, what's a genuine blocker, and what's just an idea being put out there.

The other thing I would say is to be sure to comment on positive things, as well as things you would change/suggest. It just helps transform the PR review from a "barrier to merging" into a genuine conversation about the code and approach, which everyone can learn more from.

[Spyros Argalias]

I agree. I've started using conventional comments lately and I love them.

[Cameron Thompson]

When I review, I try to follow some techniques outlined in this article:
mtlynch.io/human-code-reviews-1/

it’s a time for improvement and learning, not a time to degrade someone. I look for ways to use the PR to teach, which helps keep sports up in the long run :)

[Waylon Walker]

Love the idea of a PR being time for teaching not degrading.