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 on r? @rust-lang/<team> #249

Merged
merged 2 commits into from
Jul 12, 2021

Conversation

LeSeulArtichaut
Copy link
Contributor

Attemps to close #187.

@camelid
Copy link
Member

camelid commented Nov 23, 2020

@LeSeulArtichaut this would be great to have! Are you planning on getting back to this?

@LeSeulArtichaut
Copy link
Contributor Author

This depends on whether @Mark-Simulacrum wants to proceed with rust-lang/triagebot#433 first, but as that PR encountered some difficulties, maybe this can be revisited?

@Mark-Simulacrum
Copy link
Member

Yeah, this seems like the right approach in the interim while we wait for GitHub to resolve the problems preventing full rollout of r? for assignment in triagebot. From a quick scroll through it looks like this wants to add team groups for the existing teams (probably at least libs and compiler perhaps?) to the real files too? I guess we can do that in follow-up PRs.

@LeSeulArtichaut
Copy link
Contributor Author

This code looks really bad, I wonder who wrote it... Oh, hey me from (exactly) 9 months ago!

From a quick scroll through it looks like this wants to add team groups for the existing teams (probably at least libs and compiler perhaps?) to the real files too? I guess we can do that in follow-up PRs.

If I understand the code correctly, it first checks if a group matches the team name, so in r? @rust-lang/libs it's going to check if libs exists :

groups.get(match.group(2))

before checking if rust-lang/libs exists :

potential = groups.get("%s/%s" % (match.group(1), match.group(2)))

So I think we don't need to change the data files, because these teams already exist in

"groups": {
"all": [],
"compiler-team": ["@eddyb", "@estebank", "@matthewjasper",
"@oli-obk", "@petrochenkov", "@varkor"],
"compiler-team-contributors": ["@davidtwco", "@ecstatic-morse", "@lcnr"],
"libs": ["@sfackler", "@dtolnay", "@JoshTriplett",
"@withoutboats", "@cramertj", "@shepmaster", "@Mark-Simulacrum",
"@kennytm", "@m-ou-se"],
"infra-ci": ["@Mark-Simulacrum", "@kennytm", "@pietroalbini"],
"rustdoc": ["@jyn514", "@GuillaumeGomez", "@ollie27"]
},

@wesleywiser
Copy link
Member

I would love to see this feature added. Is there anything I can do to help get this merged? I see this needs a rebase and maybe another review?

@LeSeulArtichaut
Copy link
Contributor Author

Rebased.

highfive/newpr.py Show resolved Hide resolved
highfive/newpr.py Show resolved Hide resolved
Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes seem reasonable to me but I don't know anything about Python

@wesleywiser
Copy link
Member

@Mark-Simulacrum I think this is good to go if you want to take a look. 🙂

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 this pull request may close these issues.

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