Skip to content
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

Retire collaborators who have been inactive for a year? #18879

Closed
benjamingr opened this issue Feb 20, 2018 · 32 comments
Closed

Retire collaborators who have been inactive for a year? #18879

benjamingr opened this issue Feb 20, 2018 · 32 comments
Labels
meta Issues and PRs related to the general management of the project.

Comments

@benjamingr
Copy link
Member

benjamingr commented Feb 20, 2018

Following the TSC decision to retire some inactive collaborators here. I think we should have a clearer process for removing collaborators who have been inactive for a period of time.

I'm not really sure what prior work is going on - but I'm going to throw around a proposal to iterate on:

Once a month:

  • look for people who have been inactive* for a year, email them asking if they're still interested in being a collaborator and participating.
  • If they say "yes", do not remove them.
  • If they say "no", move them to "Collaborator Emeritus" status and remove them from the nodejs/collaborators team.
  • If they do not answer, do not remove them, if they do not answer 2 months in a row - that is like saying "no".
  • If they ever become active again - they may self nominate for collaborator. Adding them back is at the discretion of the TSC (like regular nominations) but likely won't be too problematic.

Note that the main concern in keeping people collaborators is that they have access to run CI jobs, land PRs, read moderation discussions ETC - so this is more about security than anything else. It's also about creating a smaller up-to-date approachable group of people to ping about issues. My main concern with this proposal is excluding people.

  • comments, PRs, other org repos, reviews and active WG membership count as participation for this proposal's purpose.

Pinging @nodejs/tsc and @nodejs/moderation to get the discussion going.

@benjamingr benjamingr added the meta Issues and PRs related to the general management of the project. label Feb 20, 2018
@joyeecheung
Copy link
Member

SGTM. I remember there is another issue in the build repo talking about revoking CI access from people who have not been active for three months?

@bnoordhuis
Copy link
Member

Yes, that's nodejs/build#744.

@mcollina
Copy link
Member

mcollina commented Feb 20, 2018 via email

@maclover7
Copy link
Contributor

+1 as long as criteria used will be very similar to that for being added in the first place -- comments in issues, helping out with the different WGs, etc.

@bnoordhuis
Copy link
Member

I wouldn't say that's the right way to look at it. Inactive CI accounts are security breaches in waiting. If you're not using your account, it's better to disable it.

@Trott
Copy link
Member

Trott commented Feb 20, 2018

My personal plan was to do this once a year. Basically, find all Collaborators with no commits since the release of the oldest supported release line of Node.js. (So, last year, it was since Node.js 4.0. Later this year, I was going to do it for people who haven't contributed since Node.js 6.0.) In practice, that tends to mean no commits for two years.

I appreciate wanting to document this process, but I think it should allow for a lot of messiness. Like, this was a ton of work last year, and I didn't even bother checking other repositories, comments, stuff like that. Just straight up commits. In theory, someone might be landing other people's commits but not have commits of their own, but in practice, that seems to never happen. Anyway, I ran the list by the TSC first. And emailed the Collaborators. And then moved them.

Doing something like this monthly sounds exhausting and time-consuming unless someone intends to automate large portions of the process.

I'm OK with this criteria not being documented (or documented as guidelines only, not hard rules) and leaving it to whatever engaged person wants to make it happen to do the work and consult with the TSC.

@devsnek
Copy link
Member

devsnek commented Feb 20, 2018

Doing something like this monthly sounds exhausting and time-consuming unless someone intends to automate large portions of the process.

@joyeecheung maybe this can be yet another thing in ncu?

@joyeecheung
Copy link
Member

joyeecheung commented Feb 20, 2018

@devsnek Well I already half-implemented something locally for the collaborator nomination template thing (and accidentally committed one of the GraphQL queries in another unrelated PR...), just need to find some time to flesh it out..

Edit: right, I also have a working but totally unmergable branch that walk through the collaborator list and list their latest comments in this issue tracker...that's written like, before or ~the same time get-metadata was born..

commit d9a52cb5d3a15781c184a79252186da11e74d1cf (HEAD -> check-activity)
Author: Joyee Cheung <joyeec9h3@gmail.com>
Date:   Mon Oct 23 14:05:52 2017 +0800

    check acitivities

commit 2f3843c05b09a70bddf9b4eb7699cffd6688f131
Author: Joyee Cheung <joyeec9h3@gmail.com>
Date:   Mon Oct 23 13:51:55 2017 +0800

    init

@benjamingr
Copy link
Member Author

benjamingr commented Feb 21, 2018

@joyeecheung awesome, can you also check for Reviewed-By with their email in the commit log?

Edit: I see this already works

@hashseed
Copy link
Member

CI access from people who have not been active for three months?

Some companies offer 12 weeks or more parental leave. I personally have had 10 weeks in a row. So... get child, come back to committer status gone.

@bnoordhuis
Copy link
Member

I got two days... privileges are easy to reinstate though, I don't think it'll be a big deal in practice.

@BridgeAR
Copy link
Member

I guess right now most inactive people are either inactive for a long term or they are at least active from time to time. So even if we set a period of six month it would probably almost be identical to a three month period.

Does someone know if we can verify my hypothesis easily?

@joyeecheung
Copy link
Member

joyeecheung commented Feb 22, 2018

Some companies offer 12 weeks or more parental leave. I personally have had 10 weeks in a row. So... get child, come back to committer status gone.

I think in that case the people who need to be absent for a long time can just say that in, say, the private discussion page before they start their leaves. Another upside about that is other people would know not to ping them or request reviews from them when unnecessary, and not to put something on hold until they show up.

@joyeecheung
Copy link
Member

joyeecheung commented Feb 22, 2018

I guess right now most inactive people are either inactive for a long term or they are at least active from time to time. So even if we set a period of six month it would probably almost be identical to a three month period.

My findings using the ncu-contrib tool was:

  1. 7 collaborators have not showed up in the organization for more than a year (I grabbed the stats on 02-20, so the start is 2017-02-20). That includes comments, reviews, PRs, issues in every repo under the Node.js organization. 18 if the start is 2017-09-20, 30 if it's 2017-12-20.
  2. More than 10 collaborators' last commit are adding themselves to the README (excluding new collaborators onboarded this year). 4 collaborators' last commit were authored in 2015, 31 collaborators have not authored any commit since 2017-02-20, 51 if it's 2017-09-20, 65 if it's 2017-12-20. Although I can recognize a lot of people who are still active on the issue tracer and helping out with whatever they can. So the list of "last commits" should only be used as cross references really - also, the tool does not count reviewers even though a lot of the times the work the reviewers do are no less than the author.

I prefer not to show the lists in public because that looks too confronting, although the tools is public (a PR though) and the Github API is public too.

@BridgeAR
Copy link
Member

So what is the conclusion out of these findings? What time frame should we use and how do we automate this? Do we handle someone who was very active for a long time and is then gone for a few month the same as someone who has contributed once each two or three month? Shall we use the commits, the reviews, the comments, the issues or the PRs as indicator and in what combination?

To me all of this still feels very vague (I might of course be mistaken). What I feel can be said with quite certainty is that we are going to remove between 20-30 collaborators.

Shall we have a couple of polls for these things?

@hashseed
Copy link
Member

  • Becoming a collaborator is fairly easyeven today.
  • If it's a generally applied policy with no exceptions, nobody should be offended.
  • Maybe introduce process to make reinstanting ex-collaborators even faster?

@joyeecheung
Copy link
Member

Maybe introduce process to make reinstanting ex-collaborators even faster?

Just wondering if it is really necessary to put the list in the README? Can we put the list in somewhere with less traffic, and link to there? Technically the source of truth should be in the GitHub team membership anyway, the list in the README is just for contacts AFAICT. We can also do the nominations etc. there. It also makes the README shorter and uh...the longer a document is, the less likely people are going to read it.

@joyeecheung
Copy link
Member

So what is the conclusion out of these findings?

My conclusion is: the list in the README may send a false message indicating the project is getting more help than it actually is. About 6% of the collaborators team have not shown up in the whole organization (not even left a comment) for more than a year, 16% have not shown up in half a year, and 27% have not shown up in the last three months.

What time frame should we use and how do we automate this?

I think we can start with 1 year. For example, we can use the tool to generate the list at the end of a year, look for people who have not shown up for a year (the tool can sort the data by the date of the last participation so you'll only need to look at the top of the list), draft something and post in the private asking if they still want to stay on the team (basically what the OP suggested).

Do we handle someone who was very active for a long time and is then gone for a few month the same as someone who has contributed once each two or three month? Shall we use the commits, the reviews, the comments, the issues or the PRs as indicator and in what combination?

I'd say we can just start with "1 year without any words - comments, reviews, PRs, issues - in the whole organization", and revisit later.

What I feel can be said with quite certainty is that we are going to remove between 20-30 collaborators.

If we start with the suggestion above, I believe we only need to ask 7 collaborators. Personally I don't mind keeping people who still want to stay.

@joyeecheung
Copy link
Member

joyeecheung commented Feb 24, 2018

Here is a poll:

If a collaborator have not shown up in the whole organization for [time span], reconfirm if they still want to stay, if they do not reply or say they do not wish to stay, move them to "Collaborator Emeritus". If they still want to stay, revisit after another [time span]:

  • 👍> 1 year
  • 👎> 6 months
  • 🎉> 3 months
  • ❤️> 1 month

Or:

  • 😕we should not be removing existing collaborators

@apapirovski
Copy link
Contributor

Being a collaborator seems to me mostly about CI access. If someone hasn't made a single PR or a commit in a year, I don't really see a good reason for them to remain a collaborator. They can always be reinstated if they decide to come back...

Also, if someone hasn't even made a comment (let alone a more substantial involvement) in 6 months, I see no reason for them to remain a collaborator.

So I guess that's two separate criteria... ? Probably not helpful, I know.

@addaleax
Copy link
Member

I have to say, I’m not a fan of removing existing collaborators periodically to keep access restrictions somewhat tight.

If that’s the goal, then let’s create a team for collaborators with CI access, use that for controlling access to it, and make moving in or out of that team really easy (just go to IRC or contact a TSC member directly & ask them to flip the bit).

I’m not too worried about the ability to land PRs, it’s not like that happens without anybody noticing, and especially not if it’s a collaborator that’s usually more or less inactive.

Just wondering if it is really necessary to put the list in the README?

I think it’s important to show appreciation for people who’ve put work into Node in a prominent place. That doesn’t have to happen in the current format or be restricted to collaborators, though.

@joyeecheung
Copy link
Member

joyeecheung commented Feb 24, 2018

I think it’s important to show appreciation for people who’ve put work into Node in a prominent place. That doesn’t have to happen in the current format or be restricted to collaborators, though.

I think we already cover that with AUTHORS? Also that's more like what everybody uses...IMO putting a list like that in the README is OK if the list contains < 10 people, but it does not really help if the list has > 100 entires (off-topic: even less helpful when the "Contributing to Node.js" section is after the super long lists).

@joyeecheung
Copy link
Member

joyeecheung commented Feb 24, 2018

If that’s the goal, then let’s create a team for collaborators with CI access, use that for controlling access to it, and make moving in or out of that team really easy (just go to IRC or contact a TSC member directly & ask them to flip the bit).

Sounds like a good idea to me. Maybe we can create a child team of @nodejs/collaborators ? We can give the privileges to the child team, and moves people out automatically after [time span] of inactivity. If they ever need access again, just say something in the team discussions and will be moved back.

I am curious if we ping the parent team, will everybody in the hierarchy get the notifications? They do, see: https://github.com/blog/2378-nested-teams-add-depth-to-your-team-structure

@mcollina
Copy link
Member

We are tackling two separate concerns here:

  1. signal the community that this project has not 100+ maintainers.
  2. keep a tight access to our infrastructure and project.

Trying to find a good solution for both at the same time is not the best approach.
As there is 2FA across the whole org now, I think 2 is less of a problem. We need more people helping out in landing PRs, not reducing the number of people that can do it. I'm not opposing this.

That being said, a year+ absence can be seen as a reason to ask if somebody would still want to be involved. Side note, the collaborator can "say no" and stay a collaborator. There should be a "fast path" document for a collaborator emeritus that becomes active again.

I am really 👎 on removing the list of people from the README.md.

@benjamingr
Copy link
Member Author

@addaleax

I have to say, I’m not a fan of removing existing collaborators periodically to keep access restrictions somewhat tight.

I agree with this - the proposal being bikeshedded is to ask them if they're still interested in being a collaborator or not. This is only after a year of complete inactivity and as said they can simply say "no".

@mcollina

There should be a "fast path" document for a collaborator emeritus that becomes active again.

I definitely agree with that.

I am really 👎 on removing the list of people from the README.md.

Also agreed, I don't think removing them from README really helps with anything (unless shorter readme is a goal) and I think acknowledging past contribution is important and meaningful. I know if I ever become inactive I'd appreciate it.

As there is 2FA across the whole org now, I think 2 is less of a problem.

I'm not sure, but that can be solved in a different way (emailing them they're still a collaborator and listed in the README but their GitHub access is frozen until they become active again, at which point we just add them again).

signal the community that this project has not 100+ maintainers.

I think this is also meaningful. I'd like to completely avoid cases where this causes friction with collaborators - this is why contacting the individual in private first and offering them a simple opt out (and over a month to think about it) is good.

@joyeecheung
Copy link
Member

joyeecheung commented Feb 25, 2018

Also agreed, I don't think removing them from README really helps with anything (unless shorter readme is a goal) and I think acknowledging past contribution is important and meaningful. I know if I ever become inactive I'd appreciate it.

I agree past contribution should be acknowledged, but I don't think putting a list of > 100 entries in the README is the best way to do it - half of the length of the current README comes from the lists, but I don't see why it cannot be put into a separate file being linked from README. Also I do think the README is longer than it should be, much like our CONTRIBUTING.md before it got split into different documents. Most popular repositories on GitHub have much shorter README with only succinct descriptions and links there - and I don't think I have seen any that lists these many people in the front page, they usually just put things like that in a AUTHORS file or on their website. That's probably a separate topic though, I'll draft some proposal in another issue later.

@jasnell
Copy link
Member

jasnell commented Feb 25, 2018

+1 to everything @mcollina said. For those contributors who are no longer actively contributing, I would take another step beyond asking them if they intend to contribute more and ask them also why they've chosen to be inactive. Knowing the answer to that may help inform the discussion further.

@BridgeAR
Copy link
Member

Is there any conclusion here?

@benjamingr
Copy link
Member Author

@BridgeAR not sure - @nodejs/tsc would anyone be willing to bring this up for discussion at some future meeting?

@Trott
Copy link
Member

Trott commented Apr 23, 2018

@BridgeAR not sure - @nodejs/tsc would anyone be willing to bring this up for discussion at some future meeting?

I personally think it's important that the TSC meetings avoid open-ended discussions as much as possible. So I'd prefer that there be a specific narrow proposal. And I'd prefer that the specific narrow proposal be opened as a PR and the conversation happen in the PR first before moving to a TSC meeting.

That said, as you probably know, anyone can put something on the TSC agenda. It doesn't have to be a TSC member. (But it's useful to get a TSC member to be a champion like you're trying to do!)

Speaking for myself only, my opinion hasn't changed since my original comment in #18879 (comment). I'm getting ready to do the second annual audit of Collaborator membership at the start of May. Maybe I can document the criteria/process then, although I have mixed-at-best feelings about writing anything that makes it sound like this thing I just happen to do once a year on my own is something the project is required to do or is part of some official something-or-other...

@benjamingr
Copy link
Member Author

I'll go ahead and close this then. I'll let you do your thing and let's revisit this again if anyone feels strongly about it.

@benjamingr
Copy link
Member Author

To clarify: I am aware that I can ask to put something on the TSC agenda (thanks for making sure I'm aware of it though). I was just wondering if any TSC member would like to pick this up and champion it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

No branches or pull requests