Skip to content
This repository was archived by the owner on Oct 7, 2024. It is now read-only.

added removeAccount#27

Merged
danfinlay merged 3 commits intoMetaMask:masterfrom
brunobar79:remove-account
Jul 12, 2018
Merged

added removeAccount#27
danfinlay merged 3 commits intoMetaMask:masterfrom
brunobar79:remove-account

Conversation

@brunobar79
Copy link
Copy Markdown
Contributor

Once MetaMask/eth-simple-keyring#11 gets merged I can update the version of eth-simple-keyring here

Comment thread package.json Outdated
"eth-hd-keyring": "^1.2.2",
"eth-sig-util": "^1.4.0",
"eth-simple-keyring": "^1.2.2",
"eth-simple-keyring": "github:brunobar79/eth-simple-keyring#remove-account",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ready to be changed to [email protected].

Copy link
Copy Markdown
Contributor

@danfinlay danfinlay left a comment

Choose a reason for hiding this comment

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

Looks good! Had a question, but doesn't need to block, this should work with or without the latest trezor-keyring update.

Comment thread package.json
"eth-hd-keyring": "^1.2.2",
"eth-sig-util": "^1.4.0",
"eth-simple-keyring": "^1.2.2",
"eth-simple-keyring": "^1.3.0",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we also need to include [email protected]?

Copy link
Copy Markdown
Contributor Author

@brunobar79 brunobar79 Jul 12, 2018

Choose a reason for hiding this comment

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

That's probably a good idea. Right now I'm adding the TrezorKeyring type in the metamask-controller (https://github.com/MetaMask/metamask-extension/pull/4625/files#diff-fd8e90928ae2cc670d306c0244a894d0R128)

If you're planning to add the multisig keyring here too then I should def. do it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh right, it's included by consumers. That's fine with me.

@danfinlay danfinlay merged commit 1a253ff into MetaMask:master Jul 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants