-
-
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
refactor: Update @metamask/assets-controllers
patch
#6898
Conversation
82d6165
to
de55ddb
Compare
fed0c61
to
e17be4a
Compare
cbe4c21
to
4b2febc
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.
Sorry for the delay in reviewing. Just one question.
4b2febc
to
994bab4
Compare
32cae15
to
c3e0c56
Compare
Codecov Report
@@ Coverage Diff @@
## main #6898 +/- ##
=======================================
Coverage 32.73% 32.73%
=======================================
Files 993 993
Lines 26614 26614
Branches 2083 2083
=======================================
Hits 8713 8713
Misses 17487 17487
Partials 414 414
|
c3e0c56
to
c5a3479
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
7e7deec
to
0489284
Compare
c5a3479
to
bd84729
Compare
0489284
to
3736e24
Compare
bd84729
to
61d86c2
Compare
c1f50fe
to
9a23657
Compare
The `@metamask/assets-controllers` patch added as part of the permission system implementation [1] has been updated to more closely match how the feature was implemented upstream [2]. It should be functionally equivalent. This relates to MetaMask/mobile-planning#877 [1]: #5062 [2]: MetaMask/core#1124
61d86c2
to
32e5b32
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs 34.6% 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. |
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.
🚀 🚀 🚀
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
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
label when dev review is completedQA Passed
label when QA has signed offDescription
The
@metamask/assets-controllers
patch added as part of the permission system implementation has been updated to more closely match how the feature was implemented upstream.This should be functionally equivalent except that the function signature of
addToken
has changed; theimage
andname
parameters were moved into an "options bag" parameter that also includesinteractingAddress
. All references to this method in the codebase have been updated accordingly.Issue
This relates to https://github.com/MetaMask/mobile-planning/issues/877
Checklist