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

[Repository automation] Flatten hierarchy for merge rights #36492

Open
ArthurSens opened this issue Nov 21, 2024 · 2 comments
Open

[Repository automation] Flatten hierarchy for merge rights #36492

ArthurSens opened this issue Nov 21, 2024 · 2 comments
Labels
needs triage New item requiring triage

Comments

@ArthurSens
Copy link
Member

ArthurSens commented Nov 21, 2024

Problem statement

When an external contributor works on a PR, this is the current process:

  • Contributor and reviewers work together on a PR.
  • Codeowner approves PR.
  • Collector approver adds "Ready to merge" label.
  • Collector maintainers go over PRs with the "Ready to merge" label and merge them or remove the label if a new conflict has appeared.

A symptom of this process is that code owners lack the independence to unblock community contributions. Collector approvers/maintainers are a smaller group than component code owners but have more responsibilities within the OpenTelemetry community.

Since the group of people with merge rights is small and we have a multi-layer process to let maintainers know when things should be merged, it's pretty common that PRs that are ready stay open for weeks or even months without people realizing that it's ready to merge.

This leads to a bad experience for multiple parties:

  • Contributor - who has to maintain an open PR for too long, often resolving merge conflicts several times and having to "hunt" approvers and maintainers.
  • Code owners - who feel impotent in helping the contributor besides pinging people over GitHub/Slack.
  • Approvers/Maintainers - who are the busiest people and are constantly pinged with "please merge my PR" only to find a merge conflict.

Suggested Improvements

The blame doesn't lie with any particular individual; everyone is doing what they were supposed to do. The poor contributor experience here is a symptom of a vertical hierarchy. The solution is to give more power to the lower levels (code owners) and free maintainers to do more relevant work.

Things we could do:

  • Empower code owners to merge PRs related to their components.
    or
  • Empower collector approvers to merge PRs.
    or
  • Automate the addition of the "Ready to merge" label so we can speed up the process by removing one step.

How

This is what I'd like to discuss with the community 🙂

To what degree of freedom are the maintainers happy to give code owners?
What kind of automation needs to be built to unblock the freedom we're discussing?

@ArthurSens ArthurSens added the needs triage New item requiring triage label Nov 21, 2024
@ArthurSens ArthurSens changed the title [Repository automation] Automate addition of "Ready to merge" label [Repository automation] Flatten hierarchy for merge rights Nov 21, 2024
@mx-psi
Copy link
Member

mx-psi commented Nov 22, 2024

To me the two low hanging fruit items that would help here:

  • Add a merge queue. In my experience as a maintianer the main reason I don't merge PRs even if they are ready is because the tests are not ready. If we had a merge queue, I wouldn't have to wait for that: as long as tests pass things are fine. This needs some trust in contributors but would be a huge improvement. We tried this once REQUEST: Repository maintenance on opentelemetry-collector-contrib community#1936, we had some issues that I think should be solved now, but I would rather start adding the merge queue to opentelemetry-collector-releases first.
  • The 'Ready to merge' automation you mention.

I am a bit hesitant about the other suggestions, the definitions at the community repo say that only maintainers "[Decide] on when PRs are merged to control the release scope." We can clarify this on the community repository if this is the direction we want to take, but I feel like we should first try to solve this while abiding by the existing project-wide approach.

I also feel like we should measure if these have any noticeable improvement. We can use these two dashboards for this:

@ArthurSens
Copy link
Member Author

ArthurSens commented Nov 22, 2024

We tried this once open-telemetry/community#1936, we had some issues that I think should be solved now, but I would rather start adding the merge queue to opentelemetry-collector-releases first.

After reading the issue you mentioned, it looks like there are no blockers now. Is there anything I could do to help test this in a smaller repository?

The 'Ready to merge' automation you mention.

What are the requirements for adding the label, in your opinion? Things that came to my mind:

  • Code owner approves the PR + CI is green.
  • Code owner approves the PR + CI is green + all conversations resolved.

If any of the requirements aren't met, the automation should also be able to remove the label.

I am a bit hesitant about the other suggestions, the definitions at the community repo say that only maintainers "[Decide] on when PRs are merged to control the release scope."

I feel like this definition works great for all repositories except Contrib due to its large pool of code and contributors. Of course, I can't speak for all folks, but my experience as a contributor/code owner is not the best because my PRs/PRs I approve stay open for way too long. I think I've solved more merge conflicts in this repo than the sum of all the other repositories I work on today.

I'm not entirely familiar with the process here in OTel (I've been contributing for only a few months), do we need to propose changes to the community repository before making changes here? Or are we free to work on improvements and change the guidelines later? I don't want to overstep whatever process is in place today.

With that said, let's work on the low hanging fruits for now and worry about the big changes if we feel like improvements still need to be made :)

I also feel like we should measure if these have any noticeable improvement. We can use these two dashboards for this:

Totally agree! Another thing I believe is worth measuring is "Contributor Happiness". I'm not sure how that could be measured though... Maybe the number of contributors that keep contributing after the first PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage New item requiring triage
Projects
None yet
Development

No branches or pull requests

2 participants