-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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 OnboardingController to BaseController v2 #26880
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. |
8ab4d47
to
58dbff7
Compare
58dbff7
to
a8567e6
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #26880 +/- ##
========================================
Coverage 70.12% 70.12%
========================================
Files 1426 1426
Lines 49705 49711 +6
Branches 13905 13905
========================================
+ Hits 34853 34858 +5
- Misses 14852 14853 +1 ☔ View full report in Codecov by Sentry. |
Builds ready [a8567e6]
Page Load Metrics (1757 ± 68 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [1af5b44]
Page Load Metrics (1750 ± 70 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
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.
This looks good overall! I just added a couple of small comments
Builds ready [782a17b]
Page Load Metrics (1729 ± 84 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Quality Gate passedIssues Measures |
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.
LGTM!
Pre-merge reviewer checklist
- I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
- ✅ Tested onboarding from fresh install
- ✅ Tested with an already onboarded account
- 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.
Builds ready [7aa9438]
Page Load Metrics (1800 ± 88 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
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.
LGTM!
Description
This PR updates
OnboardingController
to inherit from BaseController V2.Details about other changes are to come...
Related issues
Fixes: #25927
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist