-
-
Notifications
You must be signed in to change notification settings - Fork 242
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
Chore/metadata refactoring 2 #9262
Conversation
7542ad0
to
7eecf1e
Compare
7eecf1e
to
3507789
Compare
packages/suite/src/actions/suite/constants/metadataConstants.ts
Outdated
Show resolved
Hide resolved
const nextMetadata = data | ||
? (JSON.parse(JSON.stringify(data)) as AccountLabels) | ||
: JSON.parse(JSON.stringify(METADATA.DEFAULT_ACCOUNT_METADATA)); |
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.
Note to myself - check if as AccountLabels
can be avoided here easily
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.
probably yes. I am checking it now
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.
uh, not so easily :( it would require a typequard
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.
maybe it's a good idea to add the sanitization (and typeguard) as a followup 🤔
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.
suuuure 🤙
@mroz22 prettier check failed, other than that LGTM |
1e37370
to
93002e2
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.
looks valid and still works as expected
Description
here I continue (#9218) adding changes to make review of #9130 feasible
Commits
1c188b0 - it is not needed to sync all the keys every time. once keys are there, they never change. when migrations come to play we call these syncs relatively more often.
c351707 - moving constatns to constants. they are going to be reused in metadataActions
3049739 - add actions refactor
3507789 - unreleated, when runnign this test locally, it wouldn't clear defaults and fail on subsequent runs.