Skip to content
This repository has been archived by the owner on Mar 14, 2023. It is now read-only.

Assign a random team member with r+ priviliges on r? @rust-lang/<team> #187

Closed
Centril opened this issue Jan 5, 2019 · 4 comments · Fixed by #249
Closed

Assign a random team member with r+ priviliges on r? @rust-lang/<team> #187

Centril opened this issue Jan 5, 2019 · 4 comments · Fixed by #249

Comments

@Centril
Copy link
Contributor

Centril commented Jan 5, 2019

If someone writes:

r? @rust-lang/docs 

Then we could for example assign steveklabnik since they have r+ rights.

This mistake happens from time to time; what happens now when someone writes the above is to leave the PR without a reviewer...

cc rust-lang/rust#57357 (comment)

@Centril
Copy link
Contributor Author

Centril commented Jan 5, 2019

I'm not familiar with the code-base but... To do this, it should be sufficient to:

  1. update the regex here: https://github.com/rust-lang-nursery/highfive/blob/414fd076a3c67cfa8b0790733c0a734cd5231cb1/highfive/newpr.py#L42
  2. add a rewrite on the form @<org>/<team> in this if statement: https://github.com/rust-lang-nursery/highfive/blob/414fd076a3c67cfa8b0790733c0a734cd5231cb1/highfive/newpr.py#L416
    1. when this form is encountered, fetch the available users in that team from GH.
    2. fetch the available users with r+ rights.
    3. intersect users in i. and ii. -- if the intersection is empty, bail and don't assign anyone (possibly comment on GH with a warning).
    4. pick a user in the intersection at random.
    5. set reviewer to whatever user selected in iv.
  3. update relevant tests somewhere in this file: https://github.com/rust-lang-nursery/highfive/blob/414fd076a3c67cfa8b0790733c0a734cd5231cb1/highfive/tests/test_newpr.py#L1

cc @pietroalbini to correct mistakes I might have made or clarifications if need be.

@pietroalbini
Copy link
Member

I'm not sure if it's that useful to intersect with the list of users with r+ rights though. The list seems correct to me, cc @davidalber

@davidalber
Copy link
Collaborator

That should cover the case for new comments. You probably want to have this behavior for messages in new PRs, as well. You'll want to encapsulate all of the logic for converting the team alias to a user inside a function, possibly inside find_reviewer.

@SimonSapin
Copy link
Contributor

Would it be enough to stick to the default directory-based assignment algorithm? #241 could be an alternative if so.

when someone writes the above is to leave the PR without a reviewer

That sounds like a bug in itself, highfive should never unassign without assigning someone else.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants