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: change ownership of profile sync from notifications to identity #28901

Merged
merged 8 commits into from
Dec 4, 2024

Conversation

mathieuartu
Copy link
Contributor

@mathieuartu mathieuartu commented Dec 3, 2024

Description

This PR takes care of all the necessary changes in order to decouple "Profile Sync" from the notifications feature.
This also underlines the ownership change from @MetaMask/notifications to @MetaMask/identity.

The changes made here will help transitioning to cleaner separation of concerns for features leveraging profile sync (i.e Notifications, Account syncing...).

⚠️ This PR does not add missing tests nor introduces any changes. This is only moving files around and separating concerns.

Open in GitHub Codespaces

Related issues

Fixes #28900

Manual testing steps

No testing steps since this PR does not change the implementation of existing features.

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Copy link
Contributor

github-actions bot commented Dec 3, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot
Copy link
Collaborator

Builds ready [cc0fb07]
Page Load Metrics (2187 ± 188 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint45128541978613294
domContentLoaded148828352172388186
load150328442187391188
domInteractive24146482914
backgroundConnect85920157
firstReactRender156923126
getState1143191584321
initialActions01000
loadScripts106322901673321154
setupStore7361063
uiStartup174332682550467224
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: -1.22 KiB (-0.02%)
  • common: 0 Bytes (0.00%)

@mathieuartu mathieuartu marked this pull request as ready for review December 4, 2024 08:17
@mathieuartu mathieuartu requested review from a team as code owners December 4, 2024 08:17
@metamaskbot
Copy link
Collaborator

Builds ready [a98eb8a]
Page Load Metrics (2042 ± 148 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint40823281742559268
domContentLoaded165929312016308148
load166829582042308148
domInteractive2595432010
backgroundConnect873282110
firstReactRender159525188
getState1132831584019
initialActions01000
loadScripts123923601552264127
setupStore776172010
uiStartup188334162404377181
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: -1.22 KiB (-0.02%)
  • common: 0 Bytes (0.00%)

mirceanis
mirceanis previously approved these changes Dec 4, 2024
Copy link
Contributor

@Prithpal-Sooriya Prithpal-Sooriya left a comment

Choose a reason for hiding this comment

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

LGTM from notifications side.

.github/CODEOWNERS Outdated Show resolved Hide resolved
Co-authored-by: Mark Stacey <[email protected]>
@metamaskbot
Copy link
Collaborator

Builds ready [a1d6c08]
Page Load Metrics (2137 ± 80 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint25023481858659317
domContentLoaded18422420209317082
load18662434213716780
domInteractive266738115
backgroundConnect9163423617
firstReactRender17292232
getState126176149115
initialActions01000
loadScripts13831863162215775
setupStore7231042
uiStartup21602741246418287
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: -1.22 KiB (-0.02%)
  • common: 0 Bytes (0.00%)

@mathieuartu mathieuartu enabled auto-merge December 4, 2024 16:21
Copy link
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

I am approving the codeowner file changes, and assume the other reviews cover the other file changes

@mathieuartu mathieuartu disabled auto-merge December 4, 2024 16:45
@mathieuartu mathieuartu enabled auto-merge December 4, 2024 18:43
@metamaskbot
Copy link
Collaborator

Builds ready [f5c9fb9]
Page Load Metrics (1797 ± 55 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint16122211180012962
domContentLoaded15842100177411053
load16212128179711455
domInteractive24463063
backgroundConnect119225199
firstReactRender15261831
getState1123061433919
initialActions01000
loadScripts11661647134510249
setupStore716921
uiStartup18622663210320096
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: -1.22 KiB (-0.02%)
  • common: 0 Bytes (0.00%)

@mathieuartu mathieuartu added this pull request to the merge queue Dec 4, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [f2918f0]
Page Load Metrics (2071 ± 121 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint172025712073266128
domContentLoaded169824712029241116
load172025482071253121
domInteractive266236105
backgroundConnect995412512
firstReactRender16110312613
getState853991646933
initialActions01000
loadScripts126719181563223107
setupStore77413147
uiStartup205835182463405194
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: -1.22 KiB (-0.02%)
  • common: 0 Bytes (0.00%)

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 4, 2024
@mathieuartu mathieuartu added this pull request to the merge queue Dec 4, 2024
Merged via the queue into main with commit 79de169 Dec 4, 2024
75 checks passed
@mathieuartu mathieuartu deleted the chore/change_profile_sync_ownership branch December 4, 2024 22:23
@github-actions github-actions bot locked and limited conversation to collaborators Dec 4, 2024
@metamaskbot metamaskbot added the release-12.10.0 Issue or pull request that will be included in release 12.10.0 label Dec 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.10.0 Issue or pull request that will be included in release 12.10.0 team-identity
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[Extension] Update codebase + CODEOWNERS to reflect ownership change (notifications -> identity)
7 participants