Try โ€‚โ€‰HackMD

Migrating highfive to triagebot

This is a proposal on how to migrate highfive to triagebot.

Triagebot will be updated with the following:

  • Add support to detect r? foo to perform the same behavior as @rustbot assign foo. This is essentially https://github.com/rust-lang/triagebot/pull/433 and https://github.com/rust-lang/triagebot/pull/946.

    • This will only be enabled if the [highfive] table is in triagebot.toml. This requirement makes it possible to transition away from highfive without having the two bots fighting each other (see transition below).

    • This will use the groups and owners configuration from triagebot.toml (see below).

    • Triagebot's behavior may be altered slightly to be more similar to highfive (though probably not much). Some behaviors may be different. For example:

      • Highfive restricts the commenter to be the author, the current assignee, or a repo collaborator. I think this restriction isn't particularly necessary (as triagebot's @rustbot assign is currently unrestricted). If we have problems with abuse, that can be handled individually, or we can add restrictions later if needed.
  • Triagebot will trigger on new PRs to auto-assign using the same behavior has highfive.

  • Add configuration to triagebot.toml to configure auto-assignment on new PRs:

    โ€‹โ€‹โ€‹โ€‹# Ad-hoc groups can be configured here.
    โ€‹โ€‹โ€‹โ€‹# Although it is preferred to use real GitHub groups, very often the
    โ€‹โ€‹โ€‹โ€‹# review rotation is not the same.
    โ€‹โ€‹โ€‹โ€‹[highfive.groups]
    โ€‹โ€‹โ€‹โ€‹libs = ["@joshtriplett", "@Mark-Simulacrum", "@kenntytm", "@m-ou-se", "@thomcc"]
    
    โ€‹โ€‹โ€‹โ€‹# This specifies users, groups, or teams to assign for different paths.
    โ€‹โ€‹โ€‹โ€‹# Triagebot will use pick one person to assign.
    โ€‹โ€‹โ€‹โ€‹# Paths are gitignore-style matches.
    โ€‹โ€‹โ€‹โ€‹[highfive.owners]
    โ€‹โ€‹โ€‹โ€‹# Examples of assigning individuals.
    โ€‹โ€‹โ€‹โ€‹"Cargo.lock" = ["@Mark-Simulacrum"]
    โ€‹โ€‹โ€‹โ€‹"/library/std/src/sys/windows" = ["@ChrisDenton"]
    โ€‹โ€‹โ€‹โ€‹# Example of assigning to a group.
    โ€‹โ€‹โ€‹โ€‹"/library/std" = ["libs"]
    โ€‹โ€‹โ€‹โ€‹# Supports gitignore patterns.
    โ€‹โ€‹โ€‹โ€‹"*.js" = ["@foo"]
    โ€‹โ€‹โ€‹โ€‹# Can use real GitHub teams.
    โ€‹โ€‹โ€‹โ€‹"/src/tools/cargo" = ["@rust-lang/cargo"]
    
    โ€‹โ€‹โ€‹โ€‹# Additional features in the highfive table.
    โ€‹โ€‹โ€‹โ€‹[highfive]
    โ€‹โ€‹โ€‹โ€‹# Automatically adds these labels on new PRs.
    โ€‹โ€‹โ€‹โ€‹new-pr-labels = ["S-waiting-on-review"]
    โ€‹โ€‹โ€‹โ€‹# If `true`, then triagebot will write a comment if the PR is opened against a different branch than the default.
    โ€‹โ€‹โ€‹โ€‹expected-branch = true
    โ€‹โ€‹โ€‹โ€‹# A URL to include in the welcome message.
    โ€‹โ€‹โ€‹โ€‹contributing = "https://rustc-dev-guide.rust-lang.org/"
    

Migration

To migrate a repo, the process is roughly:

  1. Post a PR to add triagebot.toml configuration.
  2. Post a PR to remove the configuration from highfive.
  3. Very shortly after the triagebot.toml file is merged, merge the highfive change (or just remove the highfive webhook from the repo) to avoid conflicting behavior.

Alternatives considered

These are some alternatives that were considered, but not chosen.

  • Use CODEOWNERS. This is a built-in GitHub system with rather sophisticated capabilities. It uses gitignore-style syntax to choose who to "request review" from based on which files were modified.

    When a team is requested, GitHub can be configured to select a subset of the team. You can select the number of people to request. You can also choose to never select certain members. It has different algorithms: Round-robin or Load-balance. Both are rather sophisticated.

    I don't think we should use CODEOWNERS for the following reasons:

    • It only does the "request review" feature, it does not set the assignment. I think it is important to very clearly mark who is responsible for the PR via the "assignment" field. The "request review" feature is tucked away in a small part of the UI, and isn't as clear to me.
    • It requires the users to have write access to the repo.
    • It is only triggered on new PR, you can't reroll (AFAIK).
    • If multiple people match, they are all requested, which makes it a little harder to make it clear exactly who is responsible for the PR.

    See https://github.com/rust-lang/highfive/issues/72

  • Configure review groups via the teams repo.

    This is certainly a viable option. I could see a few different ways of approaching this, such as creating "real" teams, or just having a configuration in each team config that lists who is in rotation.

    This seems a bit overkill, and would require spreading the configuration between triagebot.toml and the teams repo. I think that would make it harder to configure.

    The downside is that this adds ad-hoc groups which can be confusing.

  • It was suggested to not allow defining individuals at all (and only allow teams), but I think that is over-restrictive.