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.

Sign up for GitHub

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

Jump to bottom

fix: Fix Engine context types #9566

Merged
merged 4 commits into from
May 8, 2024
Merged

fix: Fix Engine context types #9566

merged 4 commits into from
May 8, 2024

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented May 6, 2024 •
edited
Loading

Description

The context property of Engine was typed as any, effectively disabling type checking for most controller interactions. The any has been removed, restoring controller type checking.

Related issues

This problem was introduced in #6749

Manual testing steps

N/A

Screenshots/Recordings

N/A

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've completed the PR template to the best of my ability
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

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.

@Gudahtt Gudahtt force-pushed the fix-engine-types branch 5 times, most recently from c0908b6 to 3637f17 Compare May 7, 2024 12:01
@Gudahtt Gudahtt added team-mobile-platform Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels May 7, 2024
@Gudahtt Gudahtt force-pushed the fix-engine-types branch from 3637f17 to f002a75 Compare May 7, 2024 12:43
@Gudahtt Gudahtt marked this pull request as ready for review May 7, 2024 12:44
@Gudahtt Gudahtt requested review from a team as code owners May 7, 2024 12:44
@Gudahtt Gudahtt marked this pull request as draft May 7, 2024 12:44
@Gudahtt Gudahtt added the Run Smoke E2E Triggers smoke e2e on Bitrise label May 7, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 7, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: f002a75
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/f09c80c5-ba9b-4781-8104-160ae13a4247

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@Gudahtt Gudahtt marked this pull request as ready for review May 7, 2024 13:28
tommasini
tommasini previously approved these changes May 7, 2024
View reviewed changes
Copy link
Contributor

@tommasini tommasini left a comment

Choose a reason for hiding this comment

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

🔥

Gudahtt
Gudahtt commented May 7, 2024
View reviewed changes
app/core/Engine.ts Outdated Show resolved Hide resolved
Gudahtt added 2 commits May 7, 2024 17:52
@Gudahtt
fix: Fix Engine context types
455cead 
The `context` property of `Engine` was typed as `any`, effectively
disabling type checking for most controller interactions. The `any` has
been removed, restoring controller type checking.
@Gudahtt
Simplify conversionRate fallback
bbe3f7d
@Gudahtt Gudahtt force-pushed the fix-engine-types branch from f002a75 to bbe3f7d Compare May 7, 2024 20:25
@Gudahtt Gudahtt added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels May 7, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 7, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: a391838
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/85a5f533-7cfa-432d-a92a-f630a9f8ada7

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented May 8, 2024

Quality Gate Passed Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
66.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@Gudahtt Gudahtt added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels May 8, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 8, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 6e9a445
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/b92ac207-2846-440d-b2be-5b4dbf9b22b2

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@Gudahtt Gudahtt merged commit c0c3b8e into main May 8, 2024
34 of 35 checks passed
@Gudahtt Gudahtt deleted the fix-engine-types branch May 8, 2024 14:17
@github-actions github-actions bot locked and limited conversation to collaborators May 8, 2024
@metamaskbot metamaskbot added the release-7.23.0 Issue or pull request that will be included in release 7.23.0 label May 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jpuri jpuri jpuri left review comments

@legobeat legobeat legobeat approved these changes

@tommasini tommasini tommasini approved these changes

Assignees
No one assigned
Labels
release-7.23.0 Issue or pull request that will be included in release 7.23.0 Run Smoke E2E Triggers smoke e2e on Bitrise team-mobile-platform
Projects
PR review queue
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Gudahtt @jpuri @tommasini @legobeat @metamaskbot