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

Update the PR merge criteria to allow situational judgement #4227

Merged
merged 2 commits into from
Sep 26, 2024

Conversation

reyang
Copy link
Member

@reyang reyang commented Sep 25, 2024

There was a discussion among TC members about whether we should require 4 approvals for OTEPs.

Given we want to merge OTEPs into the specification repo, I think it'll be good to improve the spec PR merge criteria. Here is the thinking behind this change:

  1. Having 2 approvals seem good to me, I haven't seen cases where people try to abuse it.
  2. Increasing the number from 2 to 4 will slow down PRs for trivial/editorial changes. Many of the editorial PRs were contributed by first-time contributor to the OpenTelemetry project. I actually think we might want to speed this up rather than slowing it down.
  3. Since the project started, I think the spec maintainers (TC members) have been careful about significant changes, so we make sure people have enough time to review, discuss and share their opinions.

Changes

Clarify that spec maintainers might put additional requirements before merging the PR.

For non-trivial changes, follow the change proposal process.

* The [spec
maintainers](https://github.com/open-telemetry/community/blob/main/community-members.md#specifications-and-proto)
might make situational judgement and put additional requirements (e.g. need
more approvals, wait for the next release train, etc.).

Copy link
Member

Choose a reason for hiding this comment

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

Follow-up: Once open-telemetry/oteps#266 is approved and implemented, I think this list should explicitly mention the 4 approvals required for new OTEPs (and less for editorial fixes on merged OTEPs, if necessary) rather than this being a situational judgement call.

@carlosalberto
Copy link
Contributor

cc @jsuereth

@yurishkuro yurishkuro merged commit 6ed9226 into open-telemetry:main Sep 26, 2024
6 checks passed
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.

9 participants