Skip to content
This repository has been archived by the owner on Apr 15, 2019. It is now read-only.

Add feature to create multiple accounts - Closes #423 #613

Merged
merged 7 commits into from
Sep 20, 2018

Conversation

shuse2
Copy link
Contributor

@shuse2 shuse2 commented Sep 13, 2018

Description

Currently, account can only be created one command per one, so the flag that can create multiple account is added.

Review checklist

@shuse2 shuse2 self-assigned this Sep 13, 2018
@shuse2 shuse2 force-pushed the 423-add_multiple_account_create branch from f9d6256 to ec348a4 Compare September 13, 2018 14:38
@shuse2 shuse2 changed the title Add feature to create multiple accounts Add feature to create multiple accounts - Closes #423 Sep 14, 2018
src/commands/account/create.js Outdated Show resolved Hide resolved
src/commands/account/create.js Outdated Show resolved Hide resolved
src/commands/account/create.js Outdated Show resolved Hide resolved
src/commands/account/create.js Outdated Show resolved Hide resolved
src/commands/account/create.js Show resolved Hide resolved
test/commands/account/create.test.js Outdated Show resolved Hide resolved
test/commands/account/create.test.js Outdated Show resolved Hide resolved
Copy link
Contributor

@mitsuaki-u mitsuaki-u left a comment

Choose a reason for hiding this comment

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

Input checks

src/commands/account/create.js Outdated Show resolved Hide resolved
src/commands/account/create.js Outdated Show resolved Hide resolved
@shuse2 shuse2 dismissed stale reviews from mitsuaki-u and willclarktech September 18, 2018 12:46

addressed

@shuse2 shuse2 force-pushed the 423-add_multiple_account_create branch from d8d133d to 5b545b3 Compare September 19, 2018 14:01
@shuse2 shuse2 changed the base branch from 604-update_readme to 2.0.0 September 19, 2018 14:02
Copy link
Contributor

@willclarktech willclarktech left a comment

Choose a reason for hiding this comment

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

Two minor comments.

) {
throw new Error('Number flag must be an integer and greater than 0');
}
const accounts = Array(number)
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer new Array

sandbox.stub().returns(defaultAddress),
sandbox
.stub()
.onFirstCall()
Copy link
Contributor

Choose a reason for hiding this comment

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

.withArgs is more robust in this case.

@shuse2 shuse2 force-pushed the 423-add_multiple_account_create branch from b55fc02 to 08b34cb Compare September 20, 2018 10:04
@shuse2 shuse2 merged commit f3f20e3 into 2.0.0 Sep 20, 2018
@shuse2 shuse2 deleted the 423-add_multiple_account_create branch September 20, 2018 10:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants