I noticed we don't really have any guidelines about how a PR should proceed in our contribution docs for more specifics on process for review/merges/approvals beyond "review your own PR". I'm talking about process questions like: • Who should you request reviews from? Does this vary if you're a maintainer or not? • Do all reviewers need to wait for everyone requested to review? That doesn't seem productive given that some people may not be available. • Is there a way to demand that a particular person review before merge? Is there a way to just softly request that someone merge but not require it? • Who should perform the final merge and what conditions should be met for that? It's possible that there is already tacit consensus on our conventions for these things, but we should probably write them down. Different repos people work in (open-source or at work) have different conventions, so we should be clear about the ones at play here.

I'd love that, if it existed 🙂

I have no problem drafting that up, but I'd like to know what the convention is before going ahead. For the answer to "mandatory vs suggested" reviews, I'd suggest just using the "review requested" for "mandatory" and just @-mentioning for the soft/suggested reviews

Personally, I see that as "reviewers" vs "assignees"

that works too

What about "Explicitly _Dog_umented Process?"

.. oh please go on, I’m intrigued how far you could take this 😄

Normally, once there's at least one positive review, I merge. However, the closer I get to the core pants engine and performance sensitive code, I try to get more people to review and have some kind of consensus.

image.png

So maybe it's only while creating a PR?

ah, maybe so…

Hmm. Well it didn't work recently when I accidentally tagged Eric for a review. Clicking their name didn't remove them from the list. Maybe hitting has fixed that, or there's an issue with my use of Brave browser.

(FYI @proud-dentist-22844 Eric uses they/them pronouns 😉)

In any case, clear direction would be great. It would be nice if we can use labels or comments or something to specify how many reviews are required. Maybe we add a workflow that manages a Check that enforces reviews based on comments? Then I could comment to say I require a review from 4 people on a likely contentious PR or something like that.

I'd like to get it so we can enforce whatever policy we land on with GitHub rules. The defaults options are described here https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets/available-rules-for-rulesets#req[…]ging but I think you can write more complex stuff with a status check if needed, but you need to write a bot/workflow to do that.

Feel free to piggy back off of https://github.com/pantsbuild/pants/issues/16519#issue-1339385645