-
-
Notifications
You must be signed in to change notification settings - Fork 1.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
fix: Fix all Engine type errors #7105
Conversation
62454ee
to
3e6f9ae
Compare
3e6f9ae
to
b523431
Compare
b523431
to
fbff397
Compare
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
All errors in Engine.ts have been resolved. This file is frequently imported throughout the application, so fixing type errors here is essential for making progress with eliminating type errors. Resolving these errors required migrating the AppConstants module to TypeScript as well. A few errors were caused by dependency issues that will be resolved soon after additional dependency updates, but I made note of some cases that require further investigation and fixes. As part of this effort, a `BackgroundState` type has been added as well. This will be useful in later refactors related to controller upgrades. This relates to MetaMask/mobile-planning#1226
fbff397
to
01df7ee
Compare
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
Kudos, SonarCloud Quality Gate passed! 2 Bugs 70.5% Coverage The version of Java (11.0.20) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. |
Development & PR Process
release-xx
label to identify the PR slated for a upcoming release (will be used in release discussion)needs-dev-review
label when work is completedneeds-qa
: PR requires manual QA.No QA/E2E only
: PR does not require any manual QA effort. Prior to merging, ensure that you have successful end-to-end test runs in Bitrise.Spot check on release build
: PR does not require feature QA but needs non-automated verification. In the description section, provide test scenarios. Add screenshots, and or recordings of what was tested.QA Passed
label when QA has signed off (Only required if the PR was labeled withneeds-qa
)team-
(orexternal-contributor
label if your not a MetaMask employee)Description
All errors in Engine.ts have been resolved. This file is frequently imported throughout the application, so fixing type errors here is essential for making progress with eliminating type errors.
A few errors were caused by dependency issues that will be resolved soon after additional dependency updates, but I made note of some cases that require further investigation and fixes.
As part of this effort, a
EngineState
type has been added as well. This will be useful in later refactors related to controller upgrades.Issue
This relates to https://github.com/MetaMask/mobile-planning/issues/1226
Checklist