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

REQUEST: Repository maintenance on opentelemetry-collector-releases #2457

Closed
ArthurSens opened this issue Nov 26, 2024 · 12 comments · Fixed by #2459
Closed

REQUEST: Repository maintenance on opentelemetry-collector-releases #2457

ArthurSens opened this issue Nov 26, 2024 · 12 comments · Fixed by #2459
Assignees
Labels
area/repo-maintenance Maintenance of repos in the open-telemetry org

Comments

@ArthurSens
Copy link
Member

Affected Repository

https://github.com/open-telemetry/opentelemetry-collector-releases

Requested changes

Enable the Require merge queue setting on the branch protection rules for the main branch with "squash" merge method

Purpose

The Github documentation linked above puts it succinctly:

The merge queue provides the same benefits as the Require branches to be up to date before merging branch protection, but does not require a pull request author to update their pull request branch and wait for status checks to finish before trying to merge.

To be 100% transparent here our final goal is to enable the same configuration in https://github.com/open-telemetry/opentelemetry-collector-contrib, but we'd like to test the possible side effects of the new configuration in a smaller repository before doing so.

In the contrib repository, our goal is to improve contributor experience by accelerating merges. It's too much manual work to keep all PRs based on latest main, so the merge queue will help us here.

Expected Duration

Permanently

Repository Maintainers

  • @open-telemetry/collector-contrib-maintainers
  • @open-telemetry/collector-maintainers
@mx-psi
Copy link
Member

mx-psi commented Nov 26, 2024

I support this

@mx-psi
Copy link
Member

mx-psi commented Nov 26, 2024

Both https://github.com/open-telemetry/opentelemetry.io and https://github.com/open-telemetry/opentelemetry-js use a merge queue. I think having the same configuration as those would be reasonable here.

@trask trask self-assigned this Nov 26, 2024
@trask
Copy link
Member

trask commented Nov 26, 2024

configured, let me know if it's working out:

image

@mx-psi
Copy link
Member

mx-psi commented Nov 26, 2024

Thanks @trask! I tried merging open-telemetry/opentelemetry-collector-releases/pull/751 but it seems like there is something related to the branch protection rules that needs to be set up in a different way:
image

I don't think I am able to fix this on my own, maybe you can compare the branch protection rules that exist in the opentelemetry(.)io/opentelemetry-js repositories?

@mx-psi
Copy link
Member

mx-psi commented Nov 26, 2024

My guess is that github-merge-queue[bot] needs to be authorized to push to the main branch/to the temporary merge queue branch.

@mx-psi
Copy link
Member

mx-psi commented Nov 26, 2024

Looks like it worked now! I am closing this, but since we will be setting this up in other repositories, would you mind documenting what was done here to solve the branch protection error?

@trask
Copy link
Member

trask commented Nov 26, 2024

Looks like it was the branch protection rule on **/** that was preventing the merge queue from getting created, I unchecked this:

image

I'm going to try narrowing these permissions down (see #2459), can you give me one more PR that I can send to the merge queue afterwards?

@mx-psi
Copy link
Member

mx-psi commented Nov 26, 2024

Okay, you can try with open-telemetry/opentelemetry-collector-releases#736, just removed from the queue. I'll reopen until we get this right :)

Another interesting bit (this may be on us): we have the merge queue now, but it seems like only the "Build" workflow is running 🤔
image

https://github.com/open-telemetry/opentelemetry-collector-releases/actions/runs/12034936523/job/33552687997

Despite having multiple merge_group items after open-telemetry/opentelemetry-collector-releases/pull/755.

@mx-psi mx-psi reopened this Nov 26, 2024
@mx-psi mx-psi moved this from Done to Repo Tasks in GitHub Maintenance Nov 26, 2024
@ArthurSens
Copy link
Member Author

Looks like it was the branch protection rule on **/** that was preventing the merge queue from getting created, I unchecked this:

Yeah, they mention this requirement here.

@trask
Copy link
Member

trask commented Nov 26, 2024

ok, the configuration in #2459 looks like it's working

Another interesting bit (this may be on us): we have the merge queue now, but it seems like only the "Build" workflow is running 🤔

you may need to add merge_group to the workflows, e.g.

on:
  pull_request:
  merge_group:

@mx-psi
Copy link
Member

mx-psi commented Nov 26, 2024

ok, the configuration in #2459 looks like it's working

Another interesting bit (this may be on us): we have the merge queue now, but it seems like only the "Build" workflow is running 🤔

you may need to add merge_group to the workflows, e.g.

on:
  pull_request:
  merge_group:

This was done in open-telemetry/opentelemetry-collector-releases/pull/755, I think what matters here is that the merge queue only waits/shows the jobs marked as required. I feel like that is an independent problem, we can make a separate request for adding further required checks in the future if needed.

@mx-psi mx-psi linked a pull request Nov 26, 2024 that will close this issue
@ArthurSens
Copy link
Member Author

All good to close this one? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/repo-maintenance Maintenance of repos in the open-telemetry org
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants