GoKawiilAfter reviewing a lot of pull requests, I’ve settled on a simple default: if my comments are all nitpicks, suggestions, questions, or non-blocking issues, I leave them and approve the PR at the same time.
Here’s some detail on how it works. But first, two clarifying questions.
Why leave comments if I’m approving?
Comments show that someone has thought about the problem and the solution, and cares about what’s happening to the code. Occasionally, they offer a chance to learn or to surface misunderstandings, assumptions, or hidden risks.
I almost always leave a comment on each PR I review, even just observations: “This class is getting big, we might want to consider adding a presenter,” or praise: “Thanks for cleaning this up!”
Why approve, if I’ve left comments that I think are worth implementing?
Because I trust my team. I know that my comments will be considered, and if they’re useful, implemented.
My team is fast enough to make the change. CI runs fast enough to validate it. The time it takes to do it isn’t an obstacle.
Process Points#
Here are a couple of process points that this workflow assumes.
... continue reading