Hey, in getting more people to sign up and use
rust-lang/team, I've seen more and more people ask about who they should ping to merge the changes, because they'll ping their team leads who approved the PR, but they often aren't the same people who can merge it. Can we document who to ping? Right I think it's been defacto core (mainly pietro, & niko afaik), and could we increase the bus factor of people with merge permissions?
hmm the people with review rights should all have merge permissions, no?
Ideally maybe we could have a bot that merges when team lead(s) approve.
maybe I'm out of touch with the current setup of other teams.
@pnkfelix I mean the number of people who review
rust-lang/team repo specifically, sorry if that was unclear.
sorry i get it now
I think just core is fine for now. Once team lead approves (who realistically is likely at least somewhat on core).
@simulacrum Not working group leads, which are the larger amount of traffic on team.
I think the majority of traffic is from formation of the ice breaker and code cleanup WGs, and that'll (I suspect) die down to near-zero within days.
I personally believe that there is not enough reason to write a bot, and I would not be entirely comfortable with giving a bot that kind of power.
@simulacrum Well my preferred solution would be to just add more people who can merge. I don't think leaving it to core is any kind of solution, when core is already so overwhelmed. There are issues old enough to walk and talk blocked on core.
@XAMPPRocky the team repo is critical, because anyone with unconditional write access to it can escalate permissions up to core and infra-admins
I had an idea to ease the problem, but I didn't have time to implement it yet
basically what we want is:
so my idea was to split the team files into two files, like:
and use github's code owners feature to make the relevant team leads and core the "code owners" of their team, and just core the "code owners" of any other file in the repo
and configure github to require approval from code owners to allow merging changes
if github works the way we want (any change other than to a
people.toml requires core team approval, even if the other file changed is a
people.toml), that would allow us to safely give write access to all team and wg leads, while being assured that they can't escalate privileges
this needs more testing in a dummy repo to make sure the feature works the way we expect (I don't have much experience with it, I mostly just read the docs) and then the actual implementation work
let me know if something is not clear, and I'll try explain it better when it's not late here :)
@Pietro Albini I understand it's critical, I still think it's worth the potential risk.
I absolutely don't want any wg lead to be able to redirect firstname.lastname@example.org emails, that would be horrible
Like what you said you proposed is very complicated compared to just trusting someone.
it's not just trust in the leads themselves, but also in their security practices
and limiting the attack surface of the project as a whole
I'd love to fix the problem, but the solution needs to avoid allowing leads to escalate privileges
if you have better ideas I'm all ears :D
@Pietro Albini Well we could have a train model, similar to the website or rust with master as a protected branch.
Have a new branch called
pending, and leads can merge whatever into that, and then someone from core commits a block of changes at once instead of each one individually.
I'm not clear how that is better from a lot of small PRs getting merged as a batch
@Lokathor What you said is what I proposed so I'm not sure what you mean.
I mean I don't see how merging 5PRs to a pending branch and then merging that branch to master once is better than just merging 5PRs each to master
@Lokathor Because any lead could merge the 5 PRs, and then someone from core only has to merge and review one PR consisting of 5 commits.
It reduces the number of tasks required by someone in core from 5 to 1.
the downside of that would be increased friction when someone from core wants to merge just a single PR
as we would have to first merge into pending and then merge pending into master
potentially having to review other changes in pending in the meantime, even though we only wanted to merge a single PR
the way I see it we only have this problem if there is a large influx of new members
so we could create tooling to easily rollup PRs, and the lead of a icebreakers crew could rollup the tens of PRs related to that into a single one for core to review
like we've been doing manually these days
I feel like the increased friction is quite minimal, it's rare that any change to
rust-lang/team is urgent, and it's fine if some of the time it's a bit harder for core, when it's easier for everyone else most of the time.
I don't feel like tooling will solve this problem, that's adding more dependencies, as the tooling needs to be maintained.
to be clear, I think if someone wants to create batch PRs then that'd be fine IMO, though it may not be that much easier than just individual PR review anyway
@simulacrum Well the main part of that is having master protected, and giving leads write permissions, which someone can't just do, it has be someone on core.
Just making batch PRs wouldn't make any sense.
batch PRs are equivalent to the staging branch?
I'm not opposed to having a staging branch that is periodically merged into master by someone from core, via a PR that pings someone on core
presuming we can do that in github without granting everyone write permissions
@simulacrum Yes equivalent to that.
Well I believe we already have that situation on other repos, e.g. I can write to the
master branch of the website but not
hm, well, something to think about. I don't personally see current situation as a big problem.
@simulacrum I'm not saying it's a big problem. I don't know if we have any really big problems, we have a lot of little ones.
Sure. I guess something we could fix. Probably won't have time soon though :)