[RFC] [Bug] New pull request automatically assigns and notifies every maintainer.

[peternied]

Is your feature request related to a problem? Please describe.

When pull requests are created in OpenSearch project, no matter what they are for, I am automatically notified by GitHub and there is no way to filter pull requests that I am participating in from those that I am not actively engaged in.

This causes me to miss pull requests that are relevant to me where I have more domain knowledge & makes it hard to use the notifications systems in general, see the thread from the public slack channel 🔒.

Describe the solution you'd like

Replace Codeowners to control accepting pull requests with a GitHub action check that reads from the list of maintainers any verifies they have approved the PR. Codeowners can still be used for mapping subject matter experts that want to be notified, but isn't a requirement for every maintainer to be listed on the top level.

By modifying the branch protection rules 'maintainer has approved' check, could be required (only bypassable by an Admin), which would offer effectively the same security as Codeowners.

Describe alternatives you've considered

I've attempted using third party tools, such as Octobox, as well as configuring email rules, and changing how I use GitHub notifications, by filtering to @mentioned. All of them fall short of the optimal solution which was GitHub teams, but is unavailable.

Additional context

When pull requests are created on OpenSearch, they use a repo specific file called CODEOWNERS [1]. This file automatically assign pull request reviewers based on who is assigned to own that code. This feature worked well when combined with Teams [2], where these could have many individuals, and the team would be notified and filterable separately from individuals. E.g. I could be on a team and the pull request notification would be tracked separately from if I was requested to review a change. However, due to a policy in the OpenSearch-Project organization there are restrictions on who can be in teams - which often prevents maintainers from being allowed. At a point in time, nearly all teams were deleted from the OpenSearch project.

• [1] About GitHub CodeOwners https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners
• [2] About GitHub Teams https://docs.github.com/en/organizations/organizing-members-into-teams/about-teams
• [3] [META] Baseline MAINTAINERS, CODEOWNERS and external collaborator permissions .github#125

[peternied]

@CEHENKLE @bbarani @dblock @macohen @seanneumann @wbeckler You've been part of conversations in the past about broader code owners and repository standards. This change would -break- OpenSearch from the existing standard, but I believe this alternative option would be useful for larger scale repositories specifically OpenSearch & OpenSearch Dashboards - I'd love your feedback.

[dblock]

I think this is a well intentioned proposal.

I don't think "breaking the standard" of CODEOWNERS = MAINTAINERS is a problem. CODEOWNERS was designed precisely for the purpose that you describe. We will need to make slight adjustments in places like https://github.com/opensearch-project/project-tools where we make this assumption, but it's NBD.

On the plus side more fragmented CODEOWNERS creates siloes of expertise, but on the minus side it reduces the number of people who can grok the entire codebase.

I support letting each maintainer decide whether they want to receive only a subset of notifications by PRing their own selections of what they want to receive notifications for into CODEOWNERS.

[reta]

I used to look at most of the pull requests (at least, briefly) but for long don't have time anymore, I think fragmented CODEOWNERS, or having the pick and choose approach, would definitely help here, with pros and cons included.

[peternied]

In my free time 🤣 - I've been working on a GitHub Action [1] that can provide a replacement for the Code Owners branch check, as things cool down for the holidays I hope to make more headway in this space.

• [1] https://github.com/peternied/required-approval

[Swiddis]

As a minimal-effort solution we can also switch from listing every maintainer in codeowners to listing a team like @ opensearch-project/opensearch-core. If I understand the docs right this will stop pinging everyone in email, and it activates Github's filter for "Awaiting review from you" as opposed to "Awaiting review from you or your team".

On the plus side more fragmented CODEOWNERS creates siloes of expertise, but on the minus side it reduces the number of people who can grok the entire codebase.

Maybe a silly suggestion, but I like the idea of a randomly-assigned wildcard reviewer for each PR, in addition to an expert owner :).

The docs even mention round robin/load balancing algorithms that could do just that.

[peternied]

As a minimal-effort solution we can also switch from listing every maintainer in codeowners to listing a team like @ opensearch-project/opensearch-core.

@Swiddis its a great idea unfortunately, the OpenSearch-Project organization does not allow membership to people that are not employees of Amazon. If we did make this change many maintainers would be excluded from this option unfairly.

If we did have this option in an inclusive way - I would jump towards it in a heartbeat.

[Swiddis]

Hm, that does make it tricky... I guess there are a few alternatives I can see.

1. Instead of teams, we can assign a relatively small number of individual owners to different parts of the project. This is used in projects like CPython and TensorFlow.
2. Instead of teams we rely on PR labels to assign reviewers, this is more technically complex (e.g. using an action) but allows a lot more flexibility. For this approach I only really know of Electron, although they also combine it with a team-based Codeowners file.
3. For very large projects like Kubernetes and Rust they use a customized bot to run the show. Goes without saying that this is extremely flexible to whatever we want to do but also would be a whole project on its own.

At a glance I'm guessing 2 is a good long-term route, but there's a surprisingly few number of projects that actually do it, so I might be missing something.

I'll be pushing for trying some alternatives with my team (Currently working on this PR), it seems like the general direction will be to have the global mask be the full maintainer list to satisfy the project-tools check but assign as much code as possible to avoid using it.