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.

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

Add new snap metadata #49

Merged
merged 3 commits into from
Sep 4, 2023
Merged

Add new snap metadata #49

merged 3 commits into from
Sep 4, 2023

Conversation

danroc
Copy link
Contributor

@danroc danroc commented Aug 22, 2023

The added metadata will be visible to users when they're adding a new snap account. All the newly introduced fields are optional, as they might not be necessary for other types of snaps.

Closes: MetaMask/accounts-planning/issues/21

The added metadata will be visible to users when they're adding a new
snap account. All the newly introduced fields are optional, as they
might not be necessary for other types of snaps.
@danroc danroc requested a review from a team as a code owner August 22, 2023 08:50
@Montoya
Copy link
Contributor

Montoya commented Aug 31, 2023

I've looked at this a bunch of times and I think I like it. It would be helpful to start using this. I have a feeling we will eventually decide to change this approach and do something different with a "v2" but we can cross that bridge when we get there. I'm curious to hear @FrederikBolding's thoughts as well.

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
Co-authored-by: Frederik Bolding <[email protected]>
FrederikBolding
FrederikBolding previously approved these changes Sep 1, 2023
Copy link
Member

@FrederikBolding FrederikBolding left a comment

Choose a reason for hiding this comment

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

LGTM. Please hold off merging until @Mrtenz also looks at this

Copy link
Member

@Mrtenz Mrtenz left a comment

Choose a reason for hiding this comment

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

A support URL could be useful?

@danroc
Copy link
Contributor Author

danroc commented Sep 1, 2023

A support URL could be useful?

Could be indeed. @AlexJupiter, WDYT?

@FrederikBolding
Copy link
Member

FrederikBolding commented Sep 1, 2023

A support URL could be useful?

Could be indeed. @AlexJupiter, WDYT?

Keep in mind that this may be used by other things than just the accounts team work 😉

You don't have to use it necessarily

@AlexJupiter
Copy link
Contributor

A support URL could be useful?

Could be indeed. @AlexJupiter, WDYT?

Yes, sure, this seems like a good addition.

@Mrtenz Mrtenz merged commit b201e67 into MetaMask:main Sep 4, 2023
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants