Options to assign Fellows to PRs automatically

[Bullrich]

The request from paritytech/opstooling#245 request to get the fellows from on-chain data and assign them to the PR based on the ranking.

This is achievable, but how we do it will greatly impact (or spam) the end users

The CAPI team has provided us the solution to obtain the members of the fellowship and their ranking (with metadata included)

(Thanks, @harrysolovay, for that!)

We are still blocked until the fellowship updates the data, so it includes their GitHub account.

We have two options

Assign the fellowship members per user

When a PR is created, the system will fetch all the users from on-chain data and assign them to the PR if they are required.

This will be quite noisy because, if we need a review from, let’s say, rank 3 or above, the system will individually assign all the users who fulfill this requirement. That means that 15 individual users will be assigned to the PR.

We could do some kind of round-robin assignment, but we don’t have any way of knowing or circumvent in the case that a user is OOF or unavailable.

Pros

• Ensure the correct users are always assigned.
• Doesn’t necessarily need to reassign the users after every review (compared to teams)

  Cons

• Will spam the users a lot.
• PRs can have a huge amount (15+) of individual requested reviews if plenty of files are modified.
• We can assign by round-robin, but we can fall under the trap of assigning users who are not available and make a PR stale for a long time.
• If, for some reason, the library fails (CAPI development is currently in 0.1.0) it could block the whole process of user assignment.

  Assign the fellows to teams

  This would work the closest to the current way that PRCR works. The idea is simple:

• We fetch the users from the on-chain data.
• We assign them to the teams they belong to.
  • E.g.: A user with a rank 3 will belong to teams:
    • Rank-3
    • Rank-2
    • Rank-1
• We request the review of the team the same way that PRCR currently does.

This will help limit individual assignments (and spam) and will look cleaner in the GUI, but could also, theoretically, be circumvented by admins of the organization.

Pros

• Produces less spam
• Looks cleaner
• Encases everyone who would be required.
  • Will deal with the issue of assigning too many users to a PR.

    Cons

• Teams are private.
  • We can show in the GitHub action who are the members of each team, but this will still be us who decide to show the information (people can say it was tampered).
• Org Admins can assign/remove team members
  • A malicious admin can assign someone to be a Rank 6 without permission.
  • A rogue admin can also simply disable PRCR, or any kind of security checks (and force push) so, while they can by-pass the team assignment, that would be the smallest offend they could do.

    When to assign teams

    If we follow this step, we have three options on when to update the team members:

• Before every PRCR execution
  • Pros
    • Will fetch and update users in real time.
    • If a fellow is removed and the PR is updated, it will reflect it immediately.
  • Cons
    • Will make the check slower.
    • If, for some reason, the library fails (CAPI development is currently in 0.1.0) it could block the whole process of team assignment.
• After every push to master
  • Pros
    • It won’t block the team assignment if the library fails.
    • Almost every day, a PR is merged, so it would ensure that the teams are up-to-date.
    • We could also trigger it manually with GitHub’s UI or a web hook
      • Do we have any way to subscribe if the fellowship members change?
  • Cons
    • If a fellow is demoted, and he goes rogue, he can approve a PR before the systems downgrades his rank/removes him from the teams.
      • How realistic is this to happen?
• With a cron job
  • Same pros and cons as After every push to master.
  • We ensure that it happens at least once per day and can manually trigger it.

---

Based on the sensibility of this change, I believe that @opstooling should not be the team to decide this change, as it won’t affect us directly, compared to the members of the fellowship.

[rzadp]

I'd like to clarify one thing.

You say that assigning (all) users Will spam the users a lot, and assigning teams Produces less spam. But if the users are in those teams, they will get notification emails. So the amount of spam / notification emails, will be exactly the same in those two options?

If I get that right, then the only downside of assigning all individual users would be visual clutter in the UI, correct?

[mutantcornholio]

If I get that right, then the only downside of assigning all individual users would be visual clutter in the UI, correct?

What happens when sufficient reviews have been submitted? Should pr-custom-review unassign all other users that were assigned?

[Bullrich]

If I get that right, then the only downside of assigning all individual users would be visual clutter in the UI, correct?

Yes, as we would not be able to see the ranks unless we check the logs.

There is also the trouble of either assigning everyone or only a few.

I don't know if there is a limit with how many users can be assigned.

I'm always against cluttering a process with information because it makes us less prone to detect error.

If I get that right, then the only downside of assigning all individual users would be visual clutter in the UI, correct?

What happens when sufficient reviews have been submitted? Should pr-custom-review unassign all other users that were assigned?

That is a very good question! What should we do there?

[Bullrich]

Notes from the meeting with the @paritytech/opstooling team:

Not assign reviewers

• Must we assign reviewers?
• Can they be validated by the necessary approvals instead?

Trust CAPI

• We can trust CAPI, knowing that they will share the responsibility with us of having the system working 😉

Having an admin assign team members

• If we still need an GitHub admin to add the fellow member to the organization and give them write access, can we ask them to also add them to their rank team?
• How often are new fellows added or their ranks updated?

CI Files

• When a special file is modified (e.g.: a CI file) should we require a special team (e.g.: CI-Team) to be an additional required review aside from the normal required reviewers?

Mix Teams and Fellows

• Should we expect that a mix from both fellows and github teams will be set up in the file as required to review the PRs?

[bkchr]

As I already have commented on the initial issue: https://github.com/paritytech/opstooling/issues/245#issuecomment-1604151329

We don't need to assign any one. At least not right now. This is not a requirement.

[mutantcornholio]

Well, let's close the issue, as we aren't doing it