rfcbot-rs icon indicating copy to clipboard operation
rfcbot-rs copied to clipboard

Relax restriction that FCPs can only be started by members of the team in question

Open thomcc opened this issue 3 years ago • 7 comments

Discussed some on Zulip, I think people are in agreement that it doesn't necessarily have to be a member of the team in question to start a FCP.

That said, I'm not 100% sure what the restriction should be. Presumably not just anybody should be able to kick off an FCP (or maybe it's not a problem?). I guess also this could be a per-team decision on whether or not to allow this. (This is perhaps more trouble than its worth though, at least initially).

I can probably take this (barring it being more surprising than expected to implement), but won't be able to get started on it until this weekend.

thomcc avatar Jul 28 '22 23:07 thomcc

The restriction I was contemplating was "on some team with access to the bot", but I suspect we'd be mostly okay with just no restrictions - the team can always cancel the FCP and abuse can be handled out of band.

Mark-Simulacrum avatar Jul 28 '22 23:07 Mark-Simulacrum

Yeah, that makes sense and is mostly what I was thinking. If it's easy to add the restricting for some team, I'll do that.

thomcc avatar Jul 28 '22 23:07 thomcc

This is tricky to add in a way that respects the logic of "member of some team", since teams/members are all manually added into the DB (in the migrations), and it doesn't know about t-{libs,compiler}-contributors (as they don't have teams).

Rather than duplicate those teams, I'm going to go about it via the "the team can always cancel the FCP and abuse can be handled out of band" you suggested for now.

That said, this is not exactly ideal. The easiest alternative would be to require it be a member of some team in the rfcbot DB, and then also add t-libs-contributors (and the relevant members) to the DB[^1]. Possibly t-compiler-contributors too... and maybe other teams[^2]?

Thoughts?

[^2]: Not sure -- teams in the rfcbot DB have to be manually updated with membership changes, so at a certain point it seems like "integrate with rust-lang/team" would be a better approach. That is also a big change, though.

[^1]: This step is optional I suppose, although it would be required for this to solve the problem I currently have


Also, on Zulip @joshtriplett suggested a way to have it started as "no disposition". This seems good to have eventually, and would be beneficial for more than just this. Sadly, it seems much harder to add -- it needs new UI (probably 2 sets of checkboxes?) which seems like it might require changing a bunch of the logic for FCPs. I'm not familiar enough with this codebase to be confident in that at the moment, so I'm inclined to punt on it for now.

Until then, I think we'd just still have this be @rfcbot fcp {merge,close}, and the team performing the FCP can just cancel/reopen if they disagree with the initial FCP disposition.

Does that seem reasonable?

thomcc avatar Aug 01 '22 00:08 thomcc

I personally don't think no disposition makes sense - having concurrent yes/no votes in the rfcbot model at least is pretty confusing for everyone I suspect.

Plus, IMO the proposer should have an opinion: if they don't they probably shouldn't kick off FCP. We already essentially have yes/no via not ticking the box and leaving comments (potentially raising concerns).

I think with the successor (rustbot) we may be able to arrive at a slightly different model, where each person is contributing an independent assessment (+1, -1, maybe +/-0), but I don't think retrofitting that is really worthwhile.

I thought we already integrated with the team repo, just there's support for in-table teams too? IIRC, the types team has had fcps and we've not deployed code since they were created unless I'm forgetting something. So it may be that you can actually support any team; it may work well to do that via a new "API" call to the all@ (dynamically generated) team.

Mark-Simulacrum avatar Aug 01 '22 01:08 Mark-Simulacrum

I'm very happy with just anyone can start an FCP for now though.

Mark-Simulacrum avatar Aug 01 '22 01:08 Mark-Simulacrum

On Sun, Jul 31, 2022 at 06:03:32PM -0700, Mark Rousskov wrote:

I think with the successor (rustbot) we may be able to arrive at a slightly different model, where each person is contributing an independent assessment (+1, -1, maybe +/-0), but I don't think retrofitting that is really worthwhile.

I agree; I don't think we should try to change that in rfcbot.

joshtriplett avatar Aug 01 '22 04:08 joshtriplett

I had misunderstood then, but am glad we're on the same page.

thomcc avatar Aug 01 '22 04:08 thomcc