Reviewer access control

[indirect]

For org accounts, allow anyone in the @org/reviewers group to approve, on non-org accounts, maybe just any collaborator? Should this be configurable?

If someone on the reviewers team but who has never logged in to patronus approves, can we set the PR status to an error like "can't merge until you log in to patronus"?

[segiddins]

Doing it based on a group would mean that you couldn't configure per-repo.

[indirect]

Ugh, good point. I just hate having to manually manage lists for every repo I want to add to Homu. Could we use @org/reponame-reviewers with fallback to @org/reviewers?

[segiddins]

That just feels like a bandaid to me, when they will need to logon to patronus anyways?

[indirect]

Reviewers need to log in to patronus once ever, but we shouldn't let them nominate themselves to be reviewers. The owner of the repo needs to be able to set reviewers for the repo, and I'm already mad at Homu and I've only had to copy and paste the Bundler core team's usernames for three repos so far.

[indirect]

(Also we could maybe send an email to github users who have never logged in to patronus before when they're added as reviewers, telling them they're invited and asking them to log in so they gain approval power? ¯_(ツ)_/¯)

[segiddins]

Ah. Well right now it's all self-nomination. How do we define "owner"?

[indirect]

Has admin permission on the repo? For Homu, that means the repo is either on that person's account (and webhooks are added automatically) or the user has access to add webhooks to the repo on the org. Either way that seems like proof they are an admin... I'd be happy with anyone in @org/owners having admin/invite privs?

[indirect]

Now that @org/owners has been demoted, we'll need to use the GitHub API to figure out who has admin and/or write permissions on the repo. I think I'd like something like this:

1. User with write permission comments to approve PR
2. Patronus notices they approved and uses the GitHub API to confirm they have write access
3. If they haven't logged in to Patronus before, Patronus comments on the PR telling them it needs their permission to merge for them with a link they can use to grant those permissions
4. Patronus creates a merge commit authored by the approver

[chalkos]

Currently in #40:

1. Ok
2. Any collaborator can approve (even if they only have read access). If the repo is owned by an org, then anyone in a group that has access to that repo has permissions to approve. fix: the list of collaborators also includes a permissions field. iterate this list and continue only if the user has push (write) access
3. Unnecessary, because when the admin logged in to Patronus, it added the bot account (e.g. bundlerbot) to the repo as a contributor with write/push access. So interactions (comment, create PR, etc) will be made by the bot that was added to the repo when the admin logged in to Patronus for the first time. caveat: if an admin removes the bot from the repo, Patronus falls back to interacting with the repo as the user who made the comment. If this user never logged in to Patronus, this will not work 💥
4. Every interaction ideally uses the bot account (the only exception to that rule is when the bot no longer has permissions). The auto merge commit message terminates with #{commenter} => #{comment}, which is not the same as authoring but I think it is acceptable nonetheless.

Maybe we should discuss the fix and the caveat, maybe even before merging #40...