Hacker News Reader

Comment by shadeless | Hacker News Reader

Does it support a dynamic reviewer? In other words, instead of assigning a person who approves pull requests, it would allow merging if at least one other person with write access approved the code.

That would be the main use case for the company I work at, right now we're doing that manually by tagging and waiting for the response.

View on HN

Currently there's two ways "approval" can be determined. 1) Approval by "any" of the reviewers means it has passed. Ex. The 3 people with write-access get chosen as the reviewers, and if any 1 of those 3 approve it, then it passes. 2) Approval by "all" of the reviewers means it has passed. Ex. Of the 3 people chose as reviewers, all 3 would have to approve the PR for it to pass.

Does that make sense? I think #1 gets at the scenario that you're talking about, but right now it doesn't automatically choose the reviewers based on who has read/write/admin access. You would just choose those people manually when you set up the repo and choose whether the rule is "anyone" can approve it or "everyone" has to approve it.

I think the scenario shadeless was describing is that the author of the PR shouldn't be able to merge but anyone else with write access could. It would be my ideal scenario as well. (You could of course bypass this by doing it in Git if you really needed to.)

Gotcha, so if three users have write access then when a PR is opened by one of them, one of the other 2 users has to approve it?

If your scenario only has two users (you and one other person) then this flow can already be accomplished (the two are both reviewers, and you set approval required by "everyone" -- so you approve your own PR and then the other user has to also). If you've got more than two users, that's something we'll have to add but it sounds like it's worth doing!

Reply on news.ycombinator.com