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 the MAINTANERS.md. #190

Merged

Conversation

xuezhaojun
Copy link
Member

@xuezhaojun xuezhaojun commented Aug 4, 2024

After the PR is merged, we should also update information in cncf repo: https://github.com/cncf/foundation/blob/2a58de4421a73db2c72f4ebe0795d8533ad1901e/project-maintainers.csv?plain=1#L1004

The motivation

  • Community users should using this doc to quickly find the maintainer for their problem in a specific area of ocm.
  • Quick glice to show the company/org of every maintainer.

The 2 options we have, and Pros and Cons

After going throught the graduated projects of cncf, we have found 2 pattern of MAINTAINER list.

Type1, json or yaml format, IaC, configuration style file

For examples:

Usually, the project's infrustructure team will setup a bot or ci tool to sync these teams configuration to github teams, then each repo under the org can leverage github teams to create the codeowners file.

Using cilium for an example:

  1. it has a repo team-manager to sync the configuration files to their github org teams.
  2. it using a codeowners file instead of owners file in their main repo.

The Pros are:

  1. github teams plus codeowners can bring more fine-grained permission management.
  2. the sync from "file changes" to actual projects is smooth, no second operation are needed.

The Cons are:

  1. it's complex to setup.
  2. it needs extra effort in tooling.

Type2, markdown format, human read friendly, document style file

For examples:

Obviously, it's much easier to setup but it's not suitable when we have a lot of sub projects or work groups or teams.

The path

We will first add a type 2 MAINTAINER.md, in this step, we will discuss and figure out what teams and work-group we need to create.

Next, when we have enough members and maintainers we will invest the more automatic way -- to create a type 1 MAINTAINER list, we may even can recurit from the community.

@openshift-ci openshift-ci bot requested review from qiujian16 and yue9944882 August 4, 2024 16:35
@xuezhaojun xuezhaojun changed the title Add the MAINTANERS.md. WIP Add the MAINTANERS.md. Aug 4, 2024
@xuezhaojun
Copy link
Member Author

xuezhaojun commented Aug 4, 2024

Some questions:

@xuezhaojun xuezhaojun force-pushed the add-maintainer branch 2 times, most recently from 22ab69f to 98d676d Compare August 8, 2024 02:18
@xuezhaojun
Copy link
Member Author

/assign @qiujian16
/assign @mikeshng

MAINTAINERS.md Outdated Show resolved Hide resolved
MAINTAINERS.md Outdated Show resolved Hide resolved
MAINTAINERS.md Show resolved Hide resolved
MAINTAINERS.md Show resolved Hide resolved
MAINTAINERS.md Outdated Show resolved Hide resolved
MAINTAINERS.md Outdated Show resolved Hide resolved
MAINTAINERS.md Show resolved Hide resolved
MAINTAINERS.md Outdated Show resolved Hide resolved
MAINTAINERS.md Show resolved Hide resolved
MAINTAINERS.md Outdated
* [clusteradm](https://github.com/open-cluster-management-io/clusteradm)
* Jian Qiu

### Integrations and Offical support Addons
Copy link
Member

Choose a reason for hiding this comment

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

Integrations and Addons should be good enough

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed.

* Zhiwei Yin
* Jian Zhu
* [sdk-go](https://github.com/open-cluster-management-io/sdk-go/blob/main/OWNERS)
* Jian Qiu
Copy link
Member

Choose a reason for hiding this comment

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

we cannot have only one maintainer for this.

Copy link
Member Author

@xuezhaojun xuezhaojun Aug 8, 2024

Choose a reason for hiding this comment

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

Another name in OWNERS file is David, added in here.

Copy link
Member Author

Choose a reason for hiding this comment

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

But I wondor whether David want to do review work of this sdk-go repo, as someone from community may throw PRs and questions to him because his name is under this repo.

Copy link
Member

Choose a reason for hiding this comment

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

we might need to tweak the sdk-go owner file

@mikeshng
Copy link
Member

mikeshng commented Aug 8, 2024

Could we have the format of:

name, company, [list of repos]

to eliminate duplicated contents?

@xuezhaojun
Copy link
Member Author

xuezhaojun commented Aug 8, 2024

Could we have the format of:

name, company, [list of repos]

to eliminate duplicated contents?

@mikeshng That way we may end with duplicated list of repos.

And I'm not sure whether we want to keep the [overall list] and [maintainers in different areas] in a same page.

For example, in prometheus, it's sperated:

After we add all maintainers in the list, we shoult have toc to take a reivew

@xuezhaojun xuezhaojun requested a review from qiujian16 August 8, 2024 14:56
MAINTAINERS.md Outdated
* Zhu Jian

### Integrations and Addons
* [multicluster-controlplane](https://github.com/open-cluster-management-io/multicluster-controlplane)
Copy link
Member Author

Choose a reason for hiding this comment

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

How about add a note here to clearify the different level of maintainibility? @qiujian16

Note: We welcome all community contributors to develop their addons in or out of the ocm-io and link them here.
The maintainers of integrations and addons only need to be responsible for their own repos.

Copy link
Member

Choose a reason for hiding this comment

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

yes, I think it is ok.

Copy link
Member Author

@xuezhaojun xuezhaojun Aug 13, 2024

Choose a reason for hiding this comment

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

@qiujian16 The Integrations and Addons part is removed, added a "Note" to describe, and also link the "Integrations and Addons" section of website. Please take another look, thanks!

@xuezhaojun xuezhaojun changed the title WIP Add the MAINTANERS.md. Add the MAINTANERS.md. Aug 12, 2024
@xuezhaojun xuezhaojun force-pushed the add-maintainer branch 2 times, most recently from 6e94194 to 57b33bb Compare August 15, 2024 02:51
@mikeshng
Copy link
Member

Should we add the community repo? It's one of our pinned repos.

@xuezhaojun
Copy link
Member Author

Should we add the community repo? It's one of our pinned repos.

Good point, a new section of "Community and Docs" added.

@xuezhaojun
Copy link
Member Author

tamalsaha is added as the PR is going to merge.

Copy link
Member

@mikeshng mikeshng left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Aug 29, 2024
@qiujian16
Copy link
Member

/approve

Copy link

openshift-ci bot commented Aug 29, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiujian16, xuezhaojun

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 1e5f27e into open-cluster-management-io:main Aug 29, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants