-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
feat: re-enable account syncing #30464
Conversation
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. |
@@ -1306,7 +1305,7 @@ export default class MetamaskController extends EventEmitter { | |||
}, | |||
}, | |||
env: { | |||
isAccountSyncingEnabled: !isProduction() && isManifestV3, | |||
isAccountSyncingEnabled: isManifestV3, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably add a comment here maybe @mathieuartu.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like a "// Please do not disable"? 😬
I disagree, because in that case, we should add this to any other feature. I think it's on us to communicate more clearly internally that account syncing is here, is enabled, and is used by a lot of people.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: we should probably don't expose this config variable anymore in the extension
Builds ready [ca9b3c0]
Page Load Metrics (1659 ± 119 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
Missing release label release-12.13.0 on PR. Adding release label release-12.13.0 on PR and removing other release labels(release-12.14.0), as PR was cherry-picked in branch 12.13.0. |
Description
This PR re-enables account syncing.
This was disabled in this PR, but shouldn't have been.
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist