web-platform-tests / rfcs

web-platform-tests RFCs
75 stars 63 forks source link

RFC 25 - Maintain Membership in Code #25

Closed jugglinmike closed 4 years ago

jugglinmike commented 4 years ago

Rendered

I've filed a pull request against the proposed repository to demonstrate what the solution could look like. Hopefully, it assuages concerns about the learning curve; one doesn't need to know much about Terraform to understand the effect of the files there.

Ms2ger commented 4 years ago

This seems generally okay. Can we set up a periodical check that the github.com data is up to date with the code?

jgraham commented 4 years ago

I'm pretty worried about the extra complexity we're taking on here. I don't really want to maintain yet another automated system without being convinced that there are substantial benefits to doing so. In general adding admin team members via PR doesn't seem compelling, and the admin team is already visible to the org members. So the upside here seems like "non org members can find out who's in the admin team" and the downside is "another automated system to learn, use and maintain, and a new possibility of the data sources getting out of sync".

jugglinmike commented 4 years ago

Can we set up a periodical check that the github.com data is up to date with the code?

We can schedule a check with TravisCI, but it will only detect when members have been removed via the GitHub UI. It won't know when members are added because it doesn't query for the full member list (only for the members that have been specified in the configuration).

In general adding admin team members via PR doesn't seem compelling, and the admin team is already visible to the org members. So the upside here seems like "non org members can find out who's in the admin team"

Everyone can also find out who was on the admin team prior to today.

We can use this to manage non-admin members and also GitHub "collaborators" (users who have been granted commit rights to specific repositories). Those lists are more dynamic and a more compelling motivation for enabling pull requests.

the downside is "another automated system to learn, use and maintain

It's difficult to debate about the grade of the learning curve (which I feel to be low), but it's easier to disregard maintenance cost. Terraform is translating a set of strings to GitHub API requests. This seems just about as stable as the GitHub API. It's not set in stone, but it also doesn't seem likely to change any time soon.

and a new possibility of the data sources getting out of sync".

That's not new; our current documentation is already out of sync.

foolip commented 4 years ago

From the risks section:

This may not be appropriate for maintaining membership status of non-administrators. As of 2019-08-07, 385 people have a non-administrative membership, but very few have publicized this information. This solution does not support private membership.

That is a shame, as I was hopeful from the title that this might be a solution to https://github.com/web-platform-tests/wpt/issues/13710.

In the past I've been skeptical that a "membership as code" solution is feasible, because it's not possible to just add members, a user has to first accept becoming a member. How does Terraform deal with this, is there an "invited" state?

jugglinmike commented 4 years ago

@foolip The docs say, "When applied, an invitation will be sent to the user to become part of the organization. When destroyed, either the invitation will be cancelled or the user will be removed."

jugglinmike commented 4 years ago

At the risk of derailing (this RFC does not currently include non-admin members): is private membership a hard requirement or just happenstance?

We definitely shouldn't change policy due to a technical limitation, but I want to be sure we aren't taking an unintentional practice for granted. After all, there's a case to be made for the transparency of an entirely public membership list.

foolip commented 4 years ago

@jugglinmike it sounds then like this doesn't give any visibility into whether someone has accepted the invite, which is what would be required for https://github.com/web-platform-tests/wpt/issues/13710.

I don't think keeping org membership private is important as such, but we haven't told anyone we'd make it public so we'd have to manage that change somehow.

Don't want to be annoying, but I'll say that I don't think https://github.com/web-platform-tests/membership should have been created in the org before this RFC was done. https://github.com/web-platform-tests/editor was by accident (https://github.com/web-platform-tests/rfcs/pull/24#issuecomment-516903870) and we shouldn't keep doing that.

foolip commented 4 years ago

@web-platform-tests/wpt-core-team this RFC is two weeks old now, we should decide what to do or what additional information is needed. Nobody is ultimately responsible for this process working, so perhaps we should assign someone as the responsible person for each RFC? Any volunteers?

sideshowbarker commented 4 years ago

Nobody is ultimately responsible for this process working, so perhaps we should assign someone as the responsible person for each RFC? Any volunteers?

I’m happy to take assignments for particular RFCs, but I don’t have any strong preference about which. What should we do? Just self-assign?

zcorpan commented 4 years ago

@sideshowbarker let's discuss that in https://github.com/web-platform-tests/rfcs/issues/26

foolip commented 4 years ago

Thanks @sideshowbarker and @zcorpan!

From the information I have right now, my view is that adding tooling around admin membership doesn't solve a big enough problem to justify the maintenance of that tooling.

jugglinmike commented 4 years ago

@jugglinmike it sounds then like this doesn't give any visibility into whether someone has accepted the invite, which is what would be required for web-platform-tests/wpt#13710.

Technically yes, but in practice, there may not be very many people who have been invited without accepting. Even fewer among them will be listed as "suggested reviewers" in WPT.

foolip commented 4 years ago

There have been lots and lots of people invited who didn't accept. I've had to poke many people, and have pruned the invite list from time to time. That's why it appears to be in good shape.

jugglinmike commented 4 years ago

In that case, thank you :)

foolip commented 4 years ago

Someone needs to be on the hook to resolve this, so I've self-assigned.

Since there is disagreement, we're at the "the issue is escalated to the core team for a decision" step.

I recommend that we don't set up this infrastructure at this time. We can revisit at a later time if we see membership maintenance as a bigger problem, such that it would justify maintaining infrastructure.

@web-platform-tests/wpt-core-team do you agree? If there's some support and no concerns, I'll consider the decision made in one week. If there is disagreement among us we'll do the "≥2/3 majority vote" thing.

If we don't accept the RFC, the repository should be moved out of the web-platform-tests org.

Ms2ger commented 4 years ago

Support the proposal to do nothing at this time.

thejohnjansen commented 4 years ago

I'm OK with doing nothing at this time.

jgraham commented 4 years ago

I also agree with doing nothing for now.

sideshowbarker commented 4 years ago

No disagreement from me

zcorpan commented 4 years ago

No disagreement from me

foolip commented 4 years ago

Resolving this by closing, and I'll initiate a transfer of the repo.

foolip commented 4 years ago

Actually, I'm unable to initiate the transfer. @jugglinmike can you transfer it to bocoup or your own account?

jugglinmike commented 4 years ago

@foolip I cannot. Are you able to delete it? That would be fine with me.

foolip commented 4 years ago

Deleting it seems like a shame, if we decide to tackle reviewer membership with some tooling we might want to take a look at this or extend it. @zcorpan do you have the power to transfer?

zcorpan commented 4 years ago

Yes. Done!

foolip commented 4 years ago

Thanks @zcorpan, can you also point to the new location in case we want to revive it later?

jugglinmike commented 4 years ago

Here it is: https://github.com/bocoup/wpt-membership-demo

foolip commented 4 years ago

Thanks @jugglinmike for always looking out for new ways to improve our infra!

jugglinmike commented 4 years ago

Thanks for the thoughtful consideration :)