-
Notifications
You must be signed in to change notification settings - Fork 932
Kick off the ownership clarification #1494
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
Conversation
|
|
||
| **Status**: [Experimental](../document-status.md) | ||
|
|
||
| **Owner:** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to define what is (not) the responsibility of the Owner, and the Domain Experts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may also add a few sentences lines when domain experts assigned to a change. Are they always added (1) or are they responsible to handle the disputes (2)? I'd prefer (1).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to work on a similar PR like #745 after this PR (I think there are several steps - clarify the responsibility, document the process, improve the tooling).
Do you think this is blocking? My thinking is that we need to clarify the ownership anyways, so we don't have to be blocked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rakyll I vote for (1).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this list be synchronized with metrics approvers https://github.com/open-telemetry/community/blob/01acd5d2c39b764554ac3d87a64a18a09250e9a3/community-members.md#user-content-specifications-and-proto:~:text=Metrics%20Approvers%3A?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this list be synchronized with metrics approvers https://github.com/open-telemetry/community/blob/01acd5d2c39b764554ac3d87a64a18a09250e9a3/community-members.md#user-content-specifications-and-proto:~:text=Metrics%20Approvers%3A?
My answer would be no, approvers don't necessarily need to own the spec (e.g. if the spec is outdated, the owner should be responsible to fix it, guess this could be a separate clarification PR like #745).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we go with (1), CODEOWNERS as suggested by @tigrannajaryan below would be more automated solution. This led me to think of an approvers list.
I like the idea of explicitly listing owners - it solves an immediate issue. Maybe without overthinking the right approach we can go with just a document header as you suggest and then re-work when needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I think having explicit owners listed here could be the 1st step, once we have all the document with owners clarified, changing it to a different format (e.g. CODEOWNERS file) could be an easy job (and we can enforce that via CI).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to define what is (not) the responsibility of the Owner, and the Domain Experts.
+1 on this.
|
Should we also add the owner and experts to CODEOWNERS to explicitly assign PRs that touch the area? |
SergeyKanzhelev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it may not be a long term solution, but it may be useful now.
I think later we can explore the automation (e.g. ensure CODEOWNERS reflect the approvers by adding a simple CI script). |
|
|
||
| **Status**: [Experimental](../document-status.md) | ||
|
|
||
| **Owner:** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to define what is (not) the responsibility of the Owner, and the Domain Experts.
+1 on this.
This reverts commit 1f8e154.
Per discussion during the 03/02/2021 Spec SIG Mtg, I'm sending this PR in hope that all the spec docs would have owners/experts clarified. This could become very useful for semantic convention spec.
@rakyll please review.
@bogdandrutu @jmacd @jsuereth I put your names as the "domain experts" for metrics API spec, please approve if you'd like to be listed in the doc. If I haven't got your approval, I'll assume that you don't want to be listed here so I'll remove your name.