Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

[beta] Fix Geth account import #4643

Merged
merged 4 commits into from
Feb 23, 2017
Merged

[beta] Fix Geth account import #4643

merged 4 commits into from
Feb 23, 2017

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Feb 22, 2017

  • Fix parity_importGethAccounts (backport of RPC commit in https://github.com/ethcore/parity/pull/4641)
  • Iterate through addresses retrieved (applicable master fix ported to beta state)
  • Fix available Geth accounts not being shown (s/accounts/_accounts/, applicable fix already in master)

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. A8-backport 🕸 Pull request is already reviewed well in another branch. M7-ui labels Feb 22, 2017
.then((result) => {
console.log('result', result);
.then((gethImported) => {
console.log('result', gethImported);
Copy link
Contributor

Choose a reason for hiding this comment

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

most likely an unwanted console.log.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, was there on purpose, left in.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please use a more fitting description than "result" then.

@@ -106,7 +106,7 @@ export default class NewGeth extends Component {
api.parity
.listGethAccounts()
.then((_addresses) => {
const addresses = (addresses || []).filter((address) => !accounts[address]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, this should have been catched by the linter with _addresses being an unused variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeap. The worst was I tracked this one down in the PRs and it was fixed in master when moved to MobX store - but only after manual testing. I remember this one when I saw it, even there it was also not caught.

@derhuerst derhuerst added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 23, 2017
@arkpar arkpar merged commit e4e25b7 into beta Feb 23, 2017
@jacogr jacogr deleted the jg-geth-import-fix branch February 27, 2017 13:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-backport 🕸 Pull request is already reviewed well in another branch. A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants