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

[chore] Add "Code Owners" field to new component template #23780

Merged
merged 1 commit into from
Jul 26, 2023

Conversation

djaglowski
Copy link
Member

#20868 represents a push to ensure that all component code owners are members of the OpenTelemetry organization.

This has direct implications for our notion of "vendor-specific components", which are those that interact specifically with a particular vendor, and are contributed and maintained by a representative of a vendor. We require that the vendor representative must maintain the component, which essentially means they must be a code owner. Therefore, I am proposing that the representative must be a member of the org before the component may be accepted.

- label: If this is a vendor-specific component, I am proposing to contribute this as a representative of the vendor.
- label: This is a vendor-specific component.
- label: (If a vendor-specific component) I am proposing to contribute and maintain this as a representative of the vendor.
- label: (If a vendor-specific component) I am a member of the OpenTelemetry project.
Copy link
Member

Choose a reason for hiding this comment

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

I'm in favor of moving this check out of the vendor-specific component section and into a general section of the template. I like the idea of asking up front if the contributor is a member or not, regardless of whether they are sponsoring a vendor-specific component.

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 am of two minds on this one. The question to me is whether or not we should assume that the proposer of the component is the contributor of it. In practice, I think this is the case the vast majority of the time, but I think there might be some value in welcoming proposals from end users, non-code contributors, etc, which could then be picked up by a contributor.

For vendor-specific components, we've made this quite explicit so I don't think it's a problem.

Copy link
Member

Choose a reason for hiding this comment

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

That is fair, we should allow components to be proposed by anyone and anyone can volunteer to carry that work forward. In that case I like the idea of whoever requests to be a code owner to already be a member for the reasons listed in my other comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this checkbox needed at all? GitHub can already tell us whether they are a member of the otel project

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if being a member of the OTel community to be a code owner should be explained somewhere in a separate section in this template, to help newcomers understand why it's important to their contribution.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this checkbox needed at all? GitHub can already tell us whether they are a member of the otel project

The case for it would be so that the proposer understands the requirements.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about "I have identified one or more people who are members or aspiring members of the OpenTelemetry project who can volunteer to serve as codeowners, and am listing them in this proposal for confirmation."

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 "I have identified one or more people who are members or aspiring members of the OpenTelemetry project who can volunteer to serve as codeowners, and am listing them in this proposal for confirmation."

This sounds good. I suppose a required text field is appropriate then.

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've reworked the PR to just add a new "Code Owners" section alongside the "Sponsor" section.

- label: If this is a vendor-specific component, I am proposing to contribute this as a representative of the vendor.
- label: This is a vendor-specific component.
- label: (If a vendor-specific component) I am proposing to contribute and maintain this as a representative of the vendor.
- label: (If a vendor-specific component) I am a member of the OpenTelemetry project.
Copy link
Member

Choose a reason for hiding this comment

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

One downside of requiring membership before proposing a component is that the proposer can no longer use the contributions from creating the component as the means to membership. This provided a clear path toward membership.

A benefit of this change, though, is that is should give us more confidence going into the proposal that the user is going to maintain the component. Personally, I think Contrib is large enough at this point that more rigorous rules and expectations are necessary to ensure we can grow any further.

@djaglowski djaglowski marked this pull request as ready for review June 27, 2023 15:24
@djaglowski djaglowski requested review from a team and fatsheep9146 June 27, 2023 15:24
@@ -26,8 +26,9 @@ body:
label: Is this a vendor-specific component?
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if there's a way to clarify this requirement. We've seen a few times now where this question isn't clear to new contributors.

Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely.

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 description currently says, A vendor-specific component directly interfaces with a vendor-specific API and is expected to be maintained by a representative of the same vendor. To me this is pretty clear, but If anyone suggests clearer language I'm happy to incorporate it into this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

A separate PR seems best, just because you are trying to land something already.

- label: If this is a vendor-specific component, I am proposing to contribute this as a representative of the vendor.
- label: This is a vendor-specific component.
- label: (If a vendor-specific component) I am proposing to contribute and maintain this as a representative of the vendor.
- label: (If a vendor-specific component) I am a member of the OpenTelemetry project.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this checkbox needed at all? GitHub can already tell us whether they are a member of the otel project

@djaglowski djaglowski changed the title [chore] Propose additional requirement for vendor-specific components [chore] Add "Code Owners" field to new component template Jun 28, 2023
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jul 26, 2023
@djaglowski djaglowski merged commit 635ec0b into open-telemetry:main Jul 26, 2023
@djaglowski djaglowski deleted the vendor-member branch July 26, 2023 14:25
@github-actions github-actions bot added this to the next release milestone Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants