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 definition of core and non-core subprojects #34

Merged
merged 1 commit into from
Oct 18, 2019

Conversation

estesp
Copy link
Member

@estesp estesp commented Oct 9, 2019

Needs review and agreement from @containerd/containerd-maintainers. We have 2 projects in queue which I believe are non-core, but valuable projects to have within our GitHub organization. One of them is already in proposal here as #33

Signed-off-by: Phil Estes [email protected]

9 maintainer's LGTM required (2/3)

Copy link
Contributor

@jterry75 jterry75 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 Phil!

Copy link
Member

@mxpv mxpv left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp force-pushed the core-and-non-core branch from e242dd6 to 8f9c876 Compare October 10, 2019 13:25
@estesp estesp force-pushed the core-and-non-core branch from 8f9c876 to 679f411 Compare October 10, 2019 13:31
@estesp
Copy link
Member Author

estesp commented Oct 10, 2019

I believe I have incorporated all suggestions/clarifications/updates based on the review comments. PTAL @crosbymichael, @dmcgowan, @AkihiroSuda, and @Random-Liu.

@crosbymichael
Copy link
Member

LGTM

project, raise objections and cast their vote. Projects must be approved by at
least 66% of the current maintainers by adding their vote.
least 66% of the current maintainers.
Copy link
Member

Choose a reason for hiding this comment

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

the current core maintainers (excluding non-core)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered going through this document and adding "core" in front of the word maintainers all the way through, but that seemed like overkill. Anything to do with overall project governance always means the core maintainers as represented in the containerd/project MAINTAINERS file. Maybe I should just add a single sentence in the opening matter to that effect?

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM
just a couple nits

New core subprojects can request to be added to the containerd GitHub
organization by submitting a project proposal via public forum (a
`containerd/project` GitHub issue is the easiest way to provide this proposal).
The existing maintainers are given five business days to discuss the new
Copy link
Member

Choose a reason for hiding this comment

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

s/The existing/The existing core/

Since this topic is for adding core sub-projects...

project, raise objections and cast their vote. Projects must be approved by at
least 66% of the current maintainers by adding their vote.
least 66% of the current maintainers.
Copy link
Member

Choose a reason for hiding this comment

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

s/the current/said/


The proposal should include a proposed list of maintainers who will manage
the non-core project and provide general information on support, releases,
stability, and any additional detail useful for the containerd maintainers to
Copy link
Member

Choose a reason for hiding this comment

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

s/for the containerd maintainers//

stability, and any additional detail useful for the containerd maintainers to
understand the scope and nature of the project.

The existing maintainers are given five business days to discuss the new
Copy link
Member

Choose a reason for hiding this comment

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

same suggested edit as above.

all core and non-core projects to help contribute to project health, maintenance,
and release processes within the GitHub organization. For ease of list management,
the `MAINTAINERS` file of a non-core project will only list the non-core project
maintainers—the core maintainers of containerd will not be appended to each subproject.
Copy link
Member

Choose a reason for hiding this comment

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

This paragraph makes it clear... suggest moving to the top of the subprojects section under the title maintainer scope or something similar.

Copy link
Member

@dmcgowan dmcgowan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Random-Liu Random-Liu left a comment

Choose a reason for hiding this comment

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

LGTM

least 66% of the current maintainers.

If a project is approved, a core maintainer will add the project to the containerd
GitHub organization and provide write access for that repository to the proposed
Copy link
Member

Choose a reason for hiding this comment

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

How does one tell whether the project is core or non-core from looking at the repo?

Copy link
Member Author

Choose a reason for hiding this comment

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

the guidance provided in the README.md change in this PR would make it distinguishable as non-core projects will clearly state their status.

It is up to us as core maintainers to validate this when accepting new non-core projects into the organization.

Copy link
Member

Choose a reason for hiding this comment

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

ACK. I think it's good to make the status more obvious so that users/contributors will have the correct expectations.

The proposal should include a proposed list of maintainers who will manage
the non-core project and provide general information on support, releases,
stability, and any additional detail useful for the containerd maintainers to
understand the scope and nature of the project.
Copy link
Member

Choose a reason for hiding this comment

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

Is there any policy to remove under-maintained project?

Copy link
Member Author

@estesp estesp Oct 11, 2019

Choose a reason for hiding this comment

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

We don't have that for any kind of project within our GitHub organization. It is up to us if we want to add that kind of policy, but by default any change can be proposed and approved with 2/3rds of maintainers approval (per our standard governance process). This would mean any core maintainer can propose that an under-maintained project be removed from the organization and have a discussion and vote. I would prefer doing that in a separate PR if we think we need a clear process (and to be clear, I'm not opposed to calling it out)

Copy link
Member

@yujuhong yujuhong Oct 11, 2019

Choose a reason for hiding this comment

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

SG. I was just curious whether there were any existing policy. Just like code in contrib, I expect some non-core projects may not get enough care and attention after a while.

Copy link
Member Author

Choose a reason for hiding this comment

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

I opened #36 so we deal with this in the document in the near future. Thanks!

Copy link
Member

@yujuhong yujuhong left a comment

Choose a reason for hiding this comment

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

LGTM

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

@estesp
Copy link
Member Author

estesp commented Oct 18, 2019

We passed the required number of LGTMs and I believe all feedback was handled. Merging; anything I missed can be handled in follow-up issues/PRs

@estesp estesp merged commit 8f5d868 into containerd:master Oct 18, 2019
@estesp estesp deleted the core-and-non-core branch October 18, 2019 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.