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

Complete migration to OWNERS file #16794

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

jmhbnz
Copy link
Member

@jmhbnz jmhbnz commented Oct 18, 2023

As part of our setup for sig-etcd we intend to adopt the kubernetes project OWNERS file approach for managing repository and github org permissions automatically as code.

In #16600 we introduced the OWNERS file for etcd.

This pull request is a tidy-up to remove the old MAINTAINERS file approach.

Sub task under: #16367

Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks!

@jberkus
Copy link

jberkus commented Oct 19, 2023

We need the MAINTAINERS file for the CNCF. We're currently both a project and a SIG, and the CNCF needs us to have a MAINTAINERs file in this repo (as well as OWNERs), and only this repo. So leave that in place while updating OWNERs.

@jmhbnz jmhbnz marked this pull request as draft October 19, 2023 18:38
@ahrtr
Copy link
Member

ahrtr commented Oct 19, 2023

and only this repo

Could you clarify this? Do you mean that we only need to keep the MAINTAINERS in this repo? In other words, we can remove MAINTAINERS files from all other repositories, e.g. bbolt/MAINTAINERS, raft/MAINTAINERS, etc.?

@jberkus
Copy link

jberkus commented Oct 19, 2023

and only this repo

Could you clarify this? Do you mean that we only need to keep the MAINTAINERS in this repo? In other words, we can remove MAINTAINERS files from all other repositories, e.g. bbolt/MAINTAINERS, raft/MAINTAINERS, etc.?

That's correct. Only the one in this repo matters.

@ahrtr
Copy link
Member

ahrtr commented Oct 20, 2023

That's correct. Only the one in this repo matters.

OK, thx for the clarification.

@jmhbnz jmhbnz force-pushed the complete-ownersfile-migration branch from 0bc62cd to 97f33be Compare October 22, 2023 02:22
@jmhbnz jmhbnz marked this pull request as ready for review October 22, 2023 02:22
@jmhbnz
Copy link
Member Author

jmhbnz commented Oct 22, 2023

MAINTAINERS file will no longer be deleted by this PR. Thanks for catching this requirement @jberkus.

@jmhbnz jmhbnz force-pushed the complete-ownersfile-migration branch from 97f33be to 0fd5109 Compare October 26, 2023 07:41
@serathius
Copy link
Member

@jberkus is right that we need a file that lists project "maintainers", but it doesn't necessarily requires a MAINTAINERS file.

We can just change the file name mentioned in https://github.com/cncf/foundation/blob/main/project-maintainers.csv#L264-L269. In the file there are sigs that directly link to repo https://github.com/kubernetes/community

To make the transition complete (not necessarily in this PR) we should remove the MAINTAINERS file and change https://github.com/cncf/foundation/blob/main/project-maintainers.csv#L264-L269 to link to https://github.com/kubernetes/community/tree/master/sig-etcd#leadership

@jmhbnz
Copy link
Member Author

jmhbnz commented Oct 29, 2023

Thanks for the idea above @serathius. This pull request was discussed during the recent etcd triage meeting (08:24).

We discussed a plan which is:

  1. Establish maintainer consensus vote to:
    a. update the cncf maintainers csv file so that it refers to our new OWNERS file.

    b. add a second section to the cncf maintainers csv file to cover the sig-etcd leadership (chairs + tech leads). This is a pattern other projects have used for example Istio or Harbour. Which allows us to resolve some cncf account permission issues without having to have sig chairs defined as an approver aka maintainer in our code OWNERS file.

  2. Raise a pr to update the cncf csv file provided the vote passes.

  3. Update this pr and proceed with removing MAINTAINERS file here.

@etcd-io/maintainers-etcd can you please give a +1 on this comment or reply with either approval to proceed with this plan or any questions. Thanks! 🙏🏻

@ahrtr
Copy link
Member

ahrtr commented Oct 29, 2023

+1 Thanks all.

@serathius
Copy link
Member

+1

@jmhbnz
Copy link
Member Author

jmhbnz commented Oct 31, 2023

Gentle ping @wenjiaswe, @mitake, @ptabor, @spzala - please see the plan outlined in my comment above and provide a +1 or any questions so we can move forward with consensus.

@wenjiaswe
Copy link
Contributor

+1 thank you very much @jmhbnz !

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.

+1 Thanks @jmhbnz

@jmhbnz jmhbnz force-pushed the complete-ownersfile-migration branch from 0fd5109 to aa67cbe Compare November 4, 2023 11:02
@jmhbnz
Copy link
Member Author

jmhbnz commented Nov 8, 2023

cncf/foundation#666 has now merged so we are good to merge this one now @serathius.

@serathius serathius merged commit 38cc9f2 into etcd-io:main Nov 9, 2023
27 checks passed
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.

9 participants