Formalize the process for moving maintainers to emeritus status - #114
Formalize the process for moving maintainers to emeritus status#114epugh wants to merge 6 commits into
Conversation
Signed-off-by: Eric Pugh <epugh@opensourceconnections.com>
5977643 to
9a17398
Compare
Signed-off-by: Eric Pugh <epugh@opensourceconnections.com>
|
Adding @andrross @getsaurabh02 to this conversation. This proposal is good start I suppose and we can build from here. |
|
|
||
| Inactivity is identified via the [OpenSearch Maintainer Dashboard](https://metrics.opensearch.org/_dashboards/app/dashboards?security_tenant=global#/view/30fedc30-9ae2-11ef-a168-f19b1bbc360c?_g=(filters:!(),refreshInterval:(pause:!t,value:0),time:(from:now-30d,to:now))&_a=(description:'Shows%20data%20about%20the%20activity%20of%20maintainers%20in%20the%20OpenSearch%20Project(since%2010-12-24,%20and%20the%20technical-steering%20repo%20since%2011-7-24).%20',filters:!(),fullScreenMode:!f,options:(hidePanelTitles:!f,useMargins:!t),query:(language:kuery,query:''),timeRestore:!t,title:'OpenSearch%20Maintainer%20Dashboard',viewMode:view)). | ||
|
|
||
| The dashboard labels a maintainer with status 'Inactive' if they have no activity in the repository in the past year. |
There was a problem hiding this comment.
Wonder if 6months is enough indication to decide on inactivity compares to 1 year.
There was a problem hiding this comment.
I am okay with any number. Six months may be not enough time for quieter/smaller repos. For example, user-behavior-insights doesn't right now have a lot of activity, so a commit in ayear would indicate interest. I also don't want to cause too much churn of moving folks to emirtus just ot have then request access again, which would presumably happen on a six month cycle more than a 1 year cycle.
There was a problem hiding this comment.
i am adding some text about 1 year being default, but there may be repos where that doesn't apply.
| ## Reinstatement | ||
|
|
||
| Emeritus status is not permanent. Any past maintainer can be reinstated at any time: | ||
|
|
||
| - The returning maintainer or another current maintainer opens a pull request moving them from the Emeritus section back to the active maintainers section, and restoring `CODEOWNERS` entries. | ||
| - The [members of the admin team](https://github.com/opensearch-project/.github/blob/main/ADMINS.md) restore the maintainer's permissions upon merge. |
There was a problem hiding this comment.
I wonder if this PR should move to .github repo to extend/enhance the original document:
https://github.com/opensearch-project/.github/blob/main/RESPONSIBILITIES.md#inactivity
There was a problem hiding this comment.
We are actually moving more of the files in .github over to this repo! See opensearch-project/.github#513 and #102. In fact, I'd love any help on getting the second approval on 513 to make the migration clearer.
There was a problem hiding this comment.
Thanks, approve the file moving pr.
|
@peterzhuamazon any thoughts on the specific mechanics of the process.. I suspect we will have a FLOOD of these once we merge this PR... If this gets agreed to at the July TSC meeting, well, we have a lot of cleanup to get done in August... I would love to see this sorted out before OpenSearchCon NA in September. Can you/other admins support the work needed once we kick this process off? |
|
|
||
| 1. Open a pull request against the repository's `MAINTAINERS.md` moving themselves from the active maintainers section to the Emeritus section, and updating `CODEOWNERS` to remove their entries. | ||
| 2. Notify the [members of the admin team](https://github.com/opensearch-project/.github/blob/main/ADMINS.md), typically by opening an issue in the [opensearch-project/.github](https://github.com/opensearch-project/.github) repo, to request that their maintain-level permissions be revoked. | ||
| 3. The admin team merges the PR and revokes the permissions. |
There was a problem hiding this comment.
Will this step fix opensearch-project/opensearch-go#741 (comment) ?
There was a problem hiding this comment.
Yes, but in your specific case there was a oversight in running the process when db moved to emeritus.
There was a problem hiding this comment.
Replied here and sorry about the confusion: opensearch-project/opensearch-go#741 (comment)
andrross
left a comment
There was a problem hiding this comment.
I left some comments on the content here, but after thinking a bit I have a higher level question: do we need this document? The existing "moving on" section in the RESPONSIBILITIES doc already defines the policy. It's pretty concise, but I think it describes the policy pretty well. We just haven't implemented it consistently. I think we're free to build a process that implements that policy without adding any new policy docs.
| 1. **Self-initiated** ([Moving On](https://github.com/opensearch-project/.github/blob/main/RESPONSIBILITIES.md#moving-on)) — the maintainer voluntarily opens a pull request moving themselves to the Emeritus section of `MAINTAINERS.md`. | ||
| 2. **External** ([Inactivity](https://github.com/opensearch-project/.github/blob/main/RESPONSIBILITIES.md#inactivity)) — the admin team opens a pull request moving an inactive maintainer to the Emeritus section on their behalf. |
There was a problem hiding this comment.
Wording nitpick, but I find the "external" term here clunky and not very clear. Calling these two paths "Moving On" and "Inactivity" seems more clear to me. It also matches https://github.com/opensearch-project/technical-steering/blob/main/policies/RESPONSIBILITIES.md#removing-a-maintainer
|
|
||
| ## External: Inactivity | ||
|
|
||
| When a maintainer has become inactive without moving themselves to Emeritus, the project may move them to Emeritus on their behalf. |
There was a problem hiding this comment.
The primary reason we need this process is to improve our security posture by adhering to the principle of least privilege: maintainers credentials should be scoped to people that actively need them. We should include that explanation somewhere in this doc, either here or in the overview.
|
|
||
| ### Identifying Inactivity | ||
|
|
||
| Inactivity is identified via the [OpenSearch Maintainer Dashboard](https://metrics.opensearch.org/_dashboards/app/dashboards?security_tenant=global#/view/30fedc30-9ae2-11ef-a168-f19b1bbc360c?_g=(filters:!(),refreshInterval:(pause:!t,value:0),time:(from:now-30d,to:now))&_a=(description:'Shows%20data%20about%20the%20activity%20of%20maintainers%20in%20the%20OpenSearch%20Project(since%2010-12-24,%20and%20the%20technical-steering%20repo%20since%2011-7-24).%20',filters:!(),fullScreenMode:!f,options:(hidePanelTitles:!f,useMargins:!t),query:(language:kuery,query:''),timeRestore:!t,title:'OpenSearch%20Maintainer%20Dashboard',viewMode:view)). |
There was a problem hiding this comment.
I would define the policy in prose here. So something like "Inactivity is defined as not using maintainer privileges for a period of 1 year or more". And then link to the dashboard as the tool used to find maintainers that fall into that category. The specific tool we use may change in the future without needing to change the policy.
|
|
||
| ### Outreach | ||
|
|
||
| Before moving an inactive maintainer to Emeritus, a TSC member or member of the admin team reaches out to the maintainer privately by email to: |
There was a problem hiding this comment.
Is this necessary? How about a simpler process like:
An existing maintainer or a member of the admin team opens a PR moving the inactive maintainer to emeritus in MAINTAINERS.md and removing from CODEOWNERS with the comment:
Hey
@inactive-maintainer, we see you haven't used your maintainer privileges in the past year. If you plan on continuing to contribute, please respond here and we'll close this PR. Otherwise you'll be moved to emeritus, but you can move back to active status at any time.
Here's an example: opensearch-project/OpenSearch#21144
There was a problem hiding this comment.
i think i viewed this as more of a "big deal", like being nominated for a maintainer. Though honestly, now that I read your text, making it a "run of the mill" process is probably better. Especailly because this is on repo by repo basis, it's not "oh you are now emeritus for the entire OpenSearch Project". So yeah, +1.
| - The returning maintainer or another current maintainer opens a pull request moving them from the Emeritus section back to the active maintainers section, and restoring `CODEOWNERS` entries. | ||
| - The [members of the admin team](https://github.com/opensearch-project/.github/blob/main/ADMINS.md) restore the maintainer's permissions upon merge. | ||
|
|
||
| ## Relationship to At-Risk Status |
There was a problem hiding this comment.
It's not totally crazy to imagine a simple/stable repo with very little active development where maintainers don't need to do much. In that case we shouldn't remove maintainers below some threshold just because they haven't needed to do much of anything in the past 6 months or whatever.
So, I think what you meant to say is "inactivity" section, as the "moving on" process works already when a maintainer iniates it! It's the inactivity trigger that we don't have running well. We could just specify in the "inactivity" section of the RESPONSIBLITIES.md file that "after 1 year of no activity then the admin team will move maintainer to emeritus" and leave it at that? I guess I'm fine with that as long as it helps us get this process moving! 🤷 |
@epugh Oops, yes, I meant the "inactivity" section. I'm honestly fine with the text of the policy as it is. We just need to implement the process that starts removing inactive maintainers! What are your ideas for making that happen? |
I think we can support this by using the data directly from the metrics cluster. The initial cleanup would take some effort to get the correct member and generate large amount of PRs. But afterwards any new removal would just be as smooth as adding a new member. We are currently working on to bring Oscar GitHub agent online, so membership management can be easier by directly @ the bot later. Anyway, I think we should progress and can call outs issues in the meantime. Thanks! |
Signed-off-by: Eric Pugh <epugh@opensourceconnections.com>
|
Thanks @peterzhuamazon and @andrross for your input, I've taken that into account. Some great ideas. |
|
In terms of makign this process work (@peterzhuamazon @andrross ), well, I think that we need to pick a few people who are in the Admin team to run the rote process of "Look at the report for a repo, open up a PR per maintainer, wait seven days, then merge". I'm sure the process could be automated, but in the short term, just getting moving would be good. I would be happy to participate, however as I am not a admin, I can't actually merge the PRs.. though I could open them.... |
|
@peterzhuamazon the Bake Time check says 48 hours, but nothing has changed for 4 days... How do I get this check cleared? Not sure Bake Time is helping us..... |
AFAIK, the bake time check is currently broken. We should (IMO) remove it from the GitHub actions on this repo. Edit: Note that it's non-blocking, though, so while it annoyingly fails, it doesn't prevent us from merging changes. |
I think this is the root cause. On line https://github.com/peternied/bake-time/blob/main/action.yml#L59 modificationTime defaults to |
@msfroh you are right.. We need some approving +1's... So team, what do we think? Does this Process look approvable? |
|
@andrross mentioned on slack with me that maybe this isn't needed to move forward... And that we should just "do the work". So I opened opensearch-project/opensearch-spark#1339 as a first one. |
Signed-off-by: Eric Pugh <epugh@opensourceconnections.com>
Signed-off-by: Eric Pugh <epugh@opensourceconnections.com>
Signed-off-by: Eric Pugh <epugh@opensourceconnections.com>
|
I used the new proposed template with this PR: opensearch-project/opensearch-api-specification#1177 |
|
@epugh Two things re: opensearch-project/opensearch-api-specification#1177:
|
Thanks for the tip on |
|
My point is:
Can the message template include some language like:
Or something like that to provide direction to people who tend to their Github inbox? |
|
@epugh @sean- I think the PR template text containing explicit instructions for maintainers is a good idea. Here are the possible outcome I see:
|
|
I am pretty happy that this PR was closed by the original maintainer: opensearch-project/opensearch-api-specification#1177. Looks like the dashboard updates every 24 hours at the end of the day, so I should see there status move from INACTIVE to ACTIVE by dint of their closing the PR late tonight! |
|
Ran the full process on opensearch-project/opensearch-spark#1339 with @peterzhuamazon merging the PR. |
|
@epugh My specific issue is that this essentially restates what is already in the inactivity policy. The thing that is not stated is a PR template for the text used. What if we simplified this doc to something like the following? Per the inactivity policy, maintainers will be moved to emeritus status after an extended period of inactivity. This process will be started in the repository in question by opening a PR with the following template text: |
Maybe what we really need is not a expanded policy document, but a process document. The current document says what the policy is, but it doesn't really convey the specific steps for following the process. And we all know that it's easy to have a policy, but hard to have it followed by all. So maybe we need a toplevel Just in the last three weeks of moving two small batches of people to emeritus, I find myself constantly trying to rmemeber what the specific recipe is to follow... --> Would this get to the heart of what you want? |
@epugh Yes! What do we need in the process, something like the following?
Does it need to be more prescriptive than that? |
Maybe not, because you have the next steps. I think we also probably want to docuemnt that the metrics dashboard isnt' perfect, because it includes archived repos as inactive maintainers as well as abandoned repos that still exist. But that could be one more step that gets us to 4 steps. Are you onboard with a |
@epugh Sounds good! Maybe |
Description
This documents the "external" approach to moving inactive maintainers to emeritus.
Some questions:
Issues Resolved
Contributes towards #99
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.