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

chore: Update codeowners to match EM agreed ownership #4750

Merged
merged 5 commits into from
Oct 2, 2024

Conversation

desi
Copy link
Contributor

@desi desi commented Oct 1, 2024

Explanation

We, the MetaMask team (holistically), have recently spent time discussing code ownership around the packages in core. As a team we have aligned on which teams own which packages/controllers within core. This PR is to reflect that alignment. Additionally, in order to not make releasing challenging for the Wallet Framework (who are the stewards of the core repo) we have added this team as codeowners of the manifest file and the changelog file for all packages.

For a little context being code owners doesn’t mean you are the only ones who work in that code but rather that you will take responsibility for making sure that the code is maintained, healthy, up to date and that others contributing to it do so in a manner that is aligned with our guidelines, processes and principles.

References

  • Internal google sheets doc teams used to align can be found in the google drive
  • Additional discussions were in the #mmig-engineering-mgmt channel on slack

Checklist

  • [ n/a ] I've updated the test suite for new or updated code as appropriate
  • [ n/a ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • [ n/a ] I've highlighted breaking changes using the "BREAKING" category above as appropriate
  • [ n/a ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@desi desi force-pushed the chore/dm-update-codeowners branch 4 times, most recently from 082549a to e6a3571 Compare October 1, 2024 19:54
@desi desi force-pushed the chore/dm-update-codeowners branch from e6a3571 to 553e3b6 Compare October 1, 2024 19:57
@desi desi marked this pull request as ready for review October 1, 2024 20:49
@desi desi requested a review from a team as a code owner October 1, 2024 20:49
@desi desi requested review from a team October 1, 2024 20:56
mindofmar
mindofmar previously approved these changes Oct 1, 2024
Copy link

@mindofmar mindofmar left a comment

Choose a reason for hiding this comment

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

Product Safety looks good, cc @AugmentedMode for visibility

gantunesr
gantunesr previously approved these changes Oct 1, 2024
Copy link
Member

@gantunesr gantunesr left a comment

Choose a reason for hiding this comment

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

Approved for accounts

Mrtenz
Mrtenz previously approved these changes Oct 1, 2024
Gudahtt
Gudahtt previously approved these changes Oct 1, 2024
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

.github/CODEOWNERS Outdated Show resolved Hide resolved
mathieuartu
mathieuartu previously approved these changes Oct 2, 2024
matthewwalsh0
matthewwalsh0 previously approved these changes Oct 2, 2024
.github/CODEOWNERS Outdated Show resolved Hide resolved
.github/CODEOWNERS Outdated Show resolved Hide resolved
darkwing
darkwing previously approved these changes Oct 2, 2024
Copy link
Contributor

@darkwing darkwing left a comment

Choose a reason for hiding this comment

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

UX knows very little about the announcement-controller but having one to own is the least we can do (?).

 - Organize by team ownership
 - Update notfications controller to be snaps
 - Add snaps to permissions controller
MajorLift
MajorLift previously approved these changes Oct 2, 2024
.github/CODEOWNERS Outdated Show resolved Hide resolved
Co-authored-by: Frederik Bolding <[email protected]>
Copy link
Contributor

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@desi desi merged commit 144f90f into main Oct 2, 2024
116 checks passed
@desi desi deleted the chore/dm-update-codeowners branch October 2, 2024 16:01
MajorLift added a commit that referenced this pull request Oct 30, 2024
…le (#4876)

## Explanation

- Applies change that was intended to be included in
#4750.

## References

<!--
Are there any issues that this pull request is tied to?
Are there other links that reviewers should consult to understand these
changes better?
Are there client or consumer pull requests to adopt any breaking
changes?

For example:

* Fixes #12345
* Related to #67890
-->

## Changelog

No user-facing changes.

## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
- [ ] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.