Athens: Suggestion for reviewers?

[carolynvs]

This is a suggestion for our reviewing guidelines that I'd appreciate feedback on from other maintainers.

When reviewing a pull request, only use the Request Changes action when you want to block the PR from being merged until the author has resolved your concern and you have explicitly approved their changes.
For non-critical feedback, use the Comment action so that another maintainer can merge the PR after feedback has been addressed, without waiting for your explicit approval.

Basically what I'm trying to avoid is a common situation where one reviewer uses Request Changes and then the PR is blocked from being merged without either a maintainer overriding your review, or forcing people to wait for you to have time to circle back and approve the author's fix. It feels a bit awkward right now when a PR is held up due to small feedback such as typos.

What do you think?

[marwan-at-work]

@carolynvs that sounds good to me. Does that mean it's the merger's responsibility to make sure previous reviews are addressed?

[carolynvs]

Does that mean it's the merger's responsibility to make sure previous reviews are addressed?

Yes, which I think is a really good thing to also make clear in our docs. It helps address an earlier concern about why we wanted to keep PRs open longer because a PR was merged with open questions and request for changes.

[michalpristas]

Oh nice, i already do that :)

[robjloranger]

Sounds great

[arschles]

I'll take this one