highfive
highfive copied to clipboard
rust-lang
Ignore new comments
This removes the handling of r?.
This is replaced by https://github.com/rust-lang/triagebot/pull/433; we should land them simultaneously.
@pietroalbini This is ready for your review. The main difference is that triagebot no longer tries to be fancy about who can use r? on PRs, because we already were letting people without special permissions do so via @rustbot assign @foo. Note that on PRs we never assign rustbot itself, and will simply not do anything if you attempt to assign someone outside the GitHub org.
This PR, once deployed, will also presumably break r? on repositories that do not enable [assign] in the triagebot.toml configuration (or are not in an organization that has triagebot configured). We can either make assignment via r? or the rustbot command on PRs globally enabled (i.e. even w/o the config) or, as I would prefer, leave this as is.
I hope to soon have support for organization-wide base configuration such that we can enable e.g. assignment across all of rust-lang in triagebot ready.
This PR looks good! Thanks for working on this Mark :heart:
This PR, once deployed, will also presumably break r? on repositories that do not enable [assign] in the triagebot.toml configuration (or are not in an organization that has triagebot configured). We can either make assignment via r? or the rustbot command on PRs globally enabled (i.e. even w/o the config) or, as I would prefer, leave this as is.
I hope to soon have support for organization-wide base configuration such that we can enable e.g. assignment across all of rust-lang in triagebot ready.
I would also prefer to leave this as-is, and land configuration files only in the repositories where we want to enable r? (we might not want people to assign themselves in repos like rust-lang/rfcs).
Before landing this we'll of course need to merge https://github.com/rust-lang/triagebot/pull/433, but also land the triagebot.tomls across the repositories where r? is already enabled. We'll also need to setup the triagebot webhook in the rust-embedded organization.
Currently the repositories with r? enabled are hosted in the rust-lang and rust-embedded (cc @therealprof) organizations. In those repos, a triagebot.toml file will need to be created with the following content:
[assign]
Hm, I think there's a lot of value in having it be global. Note that in e.g. rust-lang/rfcs, r? would only work to assign organization members which seems largely fine to me.
But yes, part of the difficulty with the global configuration is that I want to support opt-out of the global configuration as well.
We'll want all repositories that highfive listens on to get a triagebot.toml with the assignment section then. I can add a key to it that gates just the r? behavior (i.e. not the "assign rustbot" piece); do we want that?
I can add a key to it that gates just the r? behavior (i.e. not the "assign rustbot" piece); do we want that?
I can't think of a repo where we would want to enable r? but not claim. If you think it's useful anyway feel free to implement that :)
You also need to change the message that highfive prints when a reviewer is assigned. Otherwise rustbot will react to highfive's r? @foo.
Okay, I've filed (and merged) PRs for all rust-lang* crates, where applicable (i.e. not deprecated/archived). Did not do so for rust-embedded; I'm a bit tired of filing the same PR :)
Will likely try to finish up sometime soon, though, and maybe some of the embedded folks could help out.
I have completely flooded this issue... I opened a PR for each rust-embedded repo mentionned in the Highfive data.
We're waiting for GitHub to implement role-based access control, as that's needed to securely give triagebot access to all the repositories without any chance of privilege escalation.
That issue was implemented about a month ago. https://github.com/github/roadmap/issues/111
