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

feat: Migrate AccountTrackerController to BaseController v2 #27258

Merged

Conversation

cryptodev-2s
Copy link
Contributor

@cryptodev-2s cryptodev-2s commented Sep 18, 2024

Description

Migrate AccountTrackerController to BaseController v2

PS: Should be merged after the conversion to typescript is merged #27231

Open in GitHub Codespaces

Related issues

Fixes: #25929

Manual testing steps

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.

@cryptodev-2s cryptodev-2s requested review from kumavis and a team as code owners September 18, 2024 15:25
Copy link
Contributor

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.

@cryptodev-2s cryptodev-2s force-pushed the feature/migrate-account-tracker-controller-to-bc2 branch from 945021d to 25d5df8 Compare September 18, 2024 15:29
@cryptodev-2s cryptodev-2s requested a review from a team September 18, 2024 15:29
@cryptodev-2s cryptodev-2s force-pushed the feature/migrate-account-tracker-controller-to-bc2 branch 2 times, most recently from 33bf4f0 to 16a5b92 Compare September 18, 2024 16:34
@cryptodev-2s cryptodev-2s added the DO-NOT-MERGE Pull requests that should not be merged label Sep 18, 2024
@desi desi linked an issue Sep 19, 2024 that may be closed by this pull request
9 tasks
@cryptodev-2s cryptodev-2s force-pushed the feature/migrate-account-tracker-controller-to-bc2 branch from 16a5b92 to a29147e Compare September 29, 2024 17:25
Base automatically changed from feature/convert-account-tracker-to-typescript to develop September 30, 2024 20:39
@cryptodev-2s cryptodev-2s force-pushed the feature/migrate-account-tracker-controller-to-bc2 branch from a29147e to 3b5a990 Compare September 30, 2024 20:47
@cryptodev-2s cryptodev-2s removed the DO-NOT-MERGE Pull requests that should not be merged label Sep 30, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [d54670c]
Page Load Metrics (1866 ± 151 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint26227731775443213
domContentLoaded15312366179920096
load153929541866314151
domInteractive15101512211
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 165 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [0e88e31]
Page Load Metrics (1752 ± 88 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14892191175518689
domContentLoaded14822162172518589
load14902193175218388
domInteractive187335136
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 165 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@MajorLift
Copy link
Contributor

The sonarcloud quality gate warning is non-blocking for now, but that will change soon, and then someone will be stuck having to make up the coverage for the changes in the PR.

In this case I guess this isn't too problematic since we expect to remove the extension controller entirely.

MajorLift
MajorLift previously approved these changes Oct 3, 2024
Copy link
Contributor

@MajorLift MajorLift left a comment

Choose a reason for hiding this comment

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

Great work as always!

Copy link
Contributor

@NiranjanaBinoy NiranjanaBinoy left a comment

Choose a reason for hiding this comment

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

LGTM

@metamaskbot
Copy link
Collaborator

Builds ready [092a237]
Page Load Metrics (1993 ± 66 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint24222691659694333
domContentLoaded16552252195813062
load16652282199313766
domInteractive28178573215
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 165 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [41139d2]
Page Load Metrics (2022 ± 120 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint24327131688707340
domContentLoaded161225041984225108
load161526852022249120
domInteractive16112422110
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 165 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@cryptodev-2s cryptodev-2s added this pull request to the merge queue Oct 4, 2024
@cryptodev-2s cryptodev-2s removed this pull request from the merge queue due to a manual request Oct 4, 2024
Copy link

sonarcloud bot commented Oct 4, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
76.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@cryptodev-2s cryptodev-2s added this pull request to the merge queue Oct 4, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [d42e196]
Page Load Metrics (2163 ± 199 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15732771567929446
domContentLoaded156932212135404194
load157732862163414199
domInteractive197852178
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 165 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Merged via the queue into develop with commit 5790f85 Oct 4, 2024
76 of 77 checks passed
@cryptodev-2s cryptodev-2s deleted the feature/migrate-account-tracker-controller-to-bc2 branch October 4, 2024 15:17
@github-actions github-actions bot locked and limited conversation to collaborators Oct 4, 2024
@metamaskbot metamaskbot added the release-12.7.0 Issue or pull request that will be included in release 12.7.0 label Oct 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.7.0 Issue or pull request that will be included in release 12.7.0 team-wallet-framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate AccountTrackerController to BaseController v2
5 participants