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 codeowners to be the actual codeowners #26795

Open
tayvano opened this issue Mar 3, 2024 · 1 comment
Open

Update codeowners to be the actual codeowners #26795

tayvano opened this issue Mar 3, 2024 · 1 comment
Labels
improvement Issue or PR for features in the software of this repo

Comments

@tayvano
Copy link
Collaborator

tayvano commented Mar 3, 2024

At some point, the codeowners of this repo were updated to "share lib engineers": #14299

This meant that the longstanding and proven group of people who previously had the ability to approve and merge PRs were thrown out and replaced by an unknown group of people.

In that PR the following statement was made:

Instead, having an owner means being someone who is responsible for things like ensuring security/maintenance issues are prioritized when they come up, or being responsible for its stability

In summary: We should use CODEOWNERS in cases where we feel a specific team should be consulted before a file is changed, rather than a statement of their ownership of a project.

While this all seems fine on the surface and I'm sure the intention was good, it turns out that (surprise surprise) people's theoretical assumptions about ownership, security, and availability do not match reality. As a result product improvements, including security improvements, have been bottlenecked due to the fact:

  1. The people on the list are not owners of this repo, are not merging PRs, have not defined a process that contibutors can follow when they want to get something shipped, and apparently do not have the [time|willingness|awareness] to do any of the above. Regardless of the root cause, this impacts the safety and security of our product offering as technical PRs aren't being reviewed, approved, merged and have not been for some time.
  2. It's unclear to the people actually contributing and maintaining this repo who those people even are. This means people don't even know who they need to sweet talk, bug, bribe to get their improvements serving our end-users.
  3. More broadly, it means that our product can only be improved by people are willing and able to navigate a hidden process and play implicitly-defined political games. This is, obviously, not ideal. Contributions should be merit-based and available to all who are willing to contribute valuable and robust code that doesn't create new security risks. There should be accountability for both those who have chosen to take on the responsibility of "owning" this repo, just as there is accountability for those actually owning and contributing to this aspect of our product.

It's obviously important that we are as safe and secure as possible, especially when it comes to our internal processes and who can merge things. Consolidating codeowners in the way we did was a good choice, especially in light of the Ledger Connect incident. However, this repo not being updated in a timely fashion also poses a risk to the safety and security of our product and that risk must be weighed alongside other risks.

I see 2 possible solutions to this problem. There may be more.

  1. The share libs engineers actually own this repo. At minimum, they need to be available and accountable. At minimum, they must prioritize PRs opened to this repo alongside their other responsibilities so that we can continue to improve one of the most impactful features of MM. At minimum, this means making themselves known to anyone who wishes to contribute to this repo and defining what a contributor needs to do in order to get their PR merged.
  2. Add the actual codeowners of this repo to the list so they can approve and merge technical PRs. Off the top of my head this includes @kumavis @409H @FrederikBolding and @samczsun and likely others.
@AlexHerman1 AlexHerman1 added the improvement Issue or PR for features in the software of this repo label Mar 4, 2024
@AlexHerman1
Copy link
Collaborator

#27941

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Issue or PR for features in the software of this repo
Projects
None yet
Development

No branches or pull requests

2 participants