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

Add process to remove maintainers that don't fulfill their duties #14238

Merged
merged 2 commits into from
Aug 23, 2022

Conversation

serathius
Copy link
Member

@serathius serathius commented Jul 19, 2022

When asking CNCF for support due to reduced community activity we stumbled upon a problem,
governing board was surprised that project is unmaintained if there is so many maintainers listed.

Feedback from CNCF governing board was to improve
accuracy of maintainers list so it reflects only active maintainers.
Self nominating process it not enough and we need to start enforcing
moving maintainers to emeritus status. Period of 12 months to take maternity leave into account.

Signed-off-by: Marek Siarkowicz siarkowicz@google.com

cc @ptabor @ahrtr @spzala @gyuho @mitake @jingyih @jpbetz @hexfusion @wenjiaswe @xiang90 @bdarnell @tbg @lilic @wilsonwang371

@serathius
Copy link
Member Author

Note, possibly we should do the same for reviewers, but first we need to define their responsibilities.

@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2022

Codecov Report

Merging #14238 (188ea54) into main (525d53b) will decrease coverage by 0.19%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #14238      +/-   ##
==========================================
- Coverage   75.35%   75.15%   -0.20%     
==========================================
  Files         456      456              
  Lines       36916    36919       +3     
==========================================
- Hits        27818    27747      -71     
- Misses       7364     7423      +59     
- Partials     1734     1749      +15     
Flag Coverage Δ
all 75.15% <ø> (-0.20%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
server/auth/simple_token.go 80.00% <0.00%> (-8.47%) ⬇️
server/etcdserver/api/v3lock/lock.go 61.53% <0.00%> (-8.47%) ⬇️
client/pkg/v3/tlsutil/tlsutil.go 83.33% <0.00%> (-8.34%) ⬇️
client/v3/experimental/recipes/queue.go 58.62% <0.00%> (-6.90%) ⬇️
client/v3/experimental/recipes/priority_queue.go 60.00% <0.00%> (-6.67%) ⬇️
client/v3/namespace/watch.go 87.87% <0.00%> (-6.07%) ⬇️
server/etcdserver/api/v2discovery/discovery.go 63.31% <0.00%> (-5.88%) ⬇️
server/lease/lease.go 94.87% <0.00%> (-5.13%) ⬇️
server/storage/mvcc/watchable_store.go 84.42% <0.00%> (-4.35%) ⬇️
client/v3/leasing/cache.go 87.77% <0.00%> (-3.89%) ⬇️
... and 19 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@hexfusion
Copy link
Contributor

In the context of removal what is the process for reactivation? Can we elaborate/flesh this out a bit?

Folks shift projects, jobs etc.

I can see the motivation for cleaning up the list.

@jpbetz
Copy link
Contributor

jpbetz commented Jul 20, 2022

Yes, I feel like I'm in the same boat as @hexfusion. I'm not active on the project and so I'd like to change status to something that reflects that, and also help ensure the active maintainers are easy to find and are appropriately recognized. I can imagine that I might return to work on the project again in the future, so having some idea how that process would work would be valuable to me, and probably quite a few others on the list.

Copy link
Member

@spzala spzala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@serathius thanks for creating this PR for discussion. My thoughts would be:

  • Maintainer(s) with 12 months of inactive period, as decided by the active maintainers, should be moved to the emeritus status after discussing this topic with them by the active maintainers. (Note, Kubernetes project use 18 months of inactive period - https://github.com/kubernetes/community/blob/master/community-membership.md#inactive-members So I would think of 12 months vs 9 months but open to other opinions)
  • If any of the emeritus maintainers wish to contribute back to the project, they will be added back to maintainers list without going through a formal process.
    @hexfusion @jpbetz

@ahrtr
Copy link
Member

ahrtr commented Jul 26, 2022

Basically I am on the same page as @spzala . The point is the existing maintainers have already demonstrated great expertise and big contribution to the community, they should be always welcome if they want to contribute back to the project.

The only minor comment is we still need to clearly clarify the process on how to add any maintainer from emeritus list to the maintainer list. If someone (assuming A) in the emeritus list want to be added back into the maintainer list, then my thought is that A is supposed to follow the process something like below,

  • Keep active in the community at least one month;
  • A doesn’t need to be nominated by any existing maintainer; Instead A can raise a issue PR in the community himself/herself, and cc all existing active (all inactive maintainers should have already been moved to the emeritus list) maintainers, and list all his/her contribution at least in the past month;
  • If there is no any objections in 3 working days (I believe there will be no any objections if A indeed has already been active for at least one month), then A will be added back to the maintainer list.

@spzala
Copy link
Member

spzala commented Jul 26, 2022

Thanks @ahrtr I agree with the overall thoughts and that we need to put this in the process. We know and trust that any emeritus maintainer/reviewer who can get back to active contributions means it, and so I think we should keep the process very simple (I am open to any more ideas but this is what I think):

  • When an emeritus maintainer(s) is back to contributing to the project and wants to be back to take the maintainer/reviewer role, they will open a PR to add themself back to the MAINTAINERS file.
  • Per two or more existing maintainer reviews or lazy consensus of a working week or three days (as you suggest), a current maintainer will merge the changes and make needed GitHub admin-related changes.

When asking CNCF for support due to reduced community activity we stumbled upon a problem,
governing board was surprised that project is unmaintained as there is so many maintainers listed for the project.

Feedback was to improve accuracy of maintainers list so it reflects only active maintainers.
Self nominating process it not enough and we need to start enforcing
moving maintainers to emeritus status. Period of 12 months to take maternity leave into account.

Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>
pick up the related work. At the very least, ensure the related work can be
continued. Afterward they can remove themselves from list of existing maintainers.

If a maintainer is not been performing their duties for period of 12 months,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

their to his/her

minor comment: I see a couple of issues related to singular vs plural:)

Copy link
Member Author

@serathius serathius Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention was to use https://en.wikipedia.org/wiki/Singular_they

It's consistent with rest of the text, for example it's used in Reviewers section.

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to me. Thanks @serathius

cc @spzala to double check the English syntax.

GOVERNANCE.md Show resolved Hide resolved
pick up the related work. At the very least, ensure the related work can be
continued. Afterward they can remove themselves from list of existing maintainers.

If a maintainer is not been performing their duties for period of 12 months,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also add why such a step is needed - "If a maintainer has not been performing their duties for period of 12 months, they can be moved to emeritus status by active maintainers to gain better understanding of recruiting need for new maintainers."

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think we should add something like this after the above sentence - "Before such a change, an inactive maintainer should be notified through email or slack by active maintainers considering GitHub notifications sometimes go unnoticed."

Copy link
Member Author

@serathius serathius Jul 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ad 1, added motivation to maintain the list to first section.

Ad 2, I think email is enough as all maintainers are required to publish their preferred email contact in ./MAINTAINERS. Activity on Slack can be disputed as we don't even have official Slack. We use K8s one as there is a lot of crossover with community.

On that topic @spzala what do you think about removing official IRC which is abandonned, for Slack (possibly CNCF or K8s one)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @serathius and yes, I agree with both the points - email is good enough, and we should remove IRC reference.

GOVERNANCE.md Outdated Show resolved Hide resolved
Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>
Copy link
Member

@spzala spzala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm @serathius thanks!

@serathius
Copy link
Member Author

Note: with @spzala approval we have started the 3 weeks lazy consensus timeout.

@serathius
Copy link
Member Author

We passed the lazy consensus time (3 business weeks). Feedback from @hexfusion and @jpbetz was also incorporated, allowing emeritus maintainers to imminently regain their status when they renew their contributions.

With 3 maintainers approval we can merge the change.

@serathius serathius merged commit e087c34 into etcd-io:main Aug 23, 2022
@ptabor
Copy link
Contributor

ptabor commented Oct 5, 2022

My small after-thought (influence also by recent issue), is that maybe we should keep RAFT maintainers as is. There is no activity, as project is considered stable/done, but when help was needed they act. In particular @tbg is advising(#14370), coding(#14413) reviewing (#14538),

@tbg
Copy link
Contributor

tbg commented Oct 31, 2022

It looks like I was removed from the etcd-maintainers project a few days ago, which means I can't approve raft PRs any more, just checking that this was intentional since the above message indicates a desire to keep me on that team.

tbg added a commit to tbg/etcd that referenced this pull request Nov 3, 2022
tbg added a commit to tbg/etcd that referenced this pull request Nov 3, 2022
As discussed with @serathius and @ptabor[^1].

[^1]: etcd-io#14238 (comment)

Signed-off-by: Tobias Grieger <tobias.b.grieger@gmail.com>
@serathius serathius deleted the emeritus branch June 15, 2023 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants