-
Notifications
You must be signed in to change notification settings - Fork 16
Remove inactive members 25-09-19 #170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
The following access changes will be introduced as a result of applying the plan: Access Changes
|
alumni: | ||
{} | ||
Alumni: | ||
members: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BigLep We could convert Alumni team members to external collaborators. The new members added in this PR haven't been active in 12 months. Where activity is defined as follows - https://github.com/filecoin-project/github-mgmt/blob/master/scripts/src/actions/remove-inactive-members.ts#L25
Before merge, verify that all the following plans are correct. They will be applied as-is after the merge. Terraform plansTerraform plans are too long to post as a comment. Please inspect Plan > Comment > Show terraform plans instead. |
A few things coming to mind here:
OR
|
For cross linking, this idea was brought up in ipdxco/github-as-code#113 |
To some extent, yes. There is no notion of conversion from member to outside collaborator in github-mgmt. We could definitely script it (there is no such thing in the provider). The missing piece is - "on membership destroy, create all missing direct collaboration links". Also, when a member is already an outside collaborator (or was converted to one through UI), they can be managed through github-mgmt just fine.
The invariant that should hold is if someone's in the Alumni team, then they are not in any other team and they are not direct collaborators of any repositories.
I just want to clarify, by putting together this PR, my intention was to see which users haven't been "active" in the org in the past 12 months (those removed from any other repositories/teams and moved to the Alumni team by the cleanup script). If we wanted to carry on with the cleanup, then, as you mentioned, we should definitely clean up this PR because it certainly was a little too eager. One thing that is pretty obviously wrong is that it moved some bot accounts to the Alumni team. That's because they don't have any activity that the script looks for.
Thank you for creating an issue for this 🙇 I think that's a really good idea.
So that's exactly what being moved to the Alumni team means. That's why I mentioned these users might be good candidates for turning into outside collaborators. As well as the current members of the Alumni team. |
archived: false | ||
collaborators: | ||
admin: | ||
- snadrus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bunch of these look like some kind of cleanup or dedupe, not removing active members? are diffs creating changes or reflecting an existing reality or just deduping individuals who already have access as a team, e.g. this one snadrus
should be in Curio
team which has admin?
This is good, but it does make it slightly harder to review. No strong opinion on whether it should be split off to a separate PR, just noting this fact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well as I read more, I'm not really sure what I would be reviewing by looking at this! So yes, a change summary of "X will lose access to Y" would certainly help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was indeed never intended for any review and this PR will be closed without being merged - it was only supposed to serve as a suggestion for which users might be up for turning into outside collaborators.
Thanks @galargh. I've been thinking about your responses and about how outside collaborators work.
I like this invariant. The problem I see though is that in order to be part of an organization team like filecoin-project/Alumni, you need to be a member of the organization, not an outside collaborator. ("Outside collaborators cannot be added to a team, team membership is restricted to members of the organization.") I agree that the users that are identified as inactive from https://github.com/filecoin-project/github-mgmt/blob/master/scripts/src/actions/remove-inactive-members.ts are good candidates to make outside collaborators. When they are made outside collaborators, they lose private repo and filecoin-project team access. It would be ideal to see what that translates to for each user, but in practice, if they haven't been active in any way for over a year, I think we're fine to blindly make them outside collaborators and let them lose the corresponding permissions. They can always be readded if needed. So it seems like the sequence of events is:
Does that seem right? I assume with Claude I can create the script easily, but if this is something you have thoughts around @galargh and want to take, that is great too. I would also want to document this process for the future. I can at least handle the documentation part if the approach seems right. |
Yes, that's correct. I was only trying to suggest that the users who are now in the
Exactly! That's why I thought putting this together could be helpful to you in the short term.
Yeah, that sounds about right. However, I would definitely advise a human step between getting a list of usernames and calling the API.
Nothing off the top of my head, but maybe something could come up in the process of making it. I can create it for you or you can have a go, just let me know :) |
Summary
Why do you need this?
What else do we need to know?
DRI: myself
Reviewer's Checklist