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

Fix tx-status to handle account id better #189

Merged
merged 2 commits into from
Nov 16, 2019
Merged

Fix tx-status to handle account id better #189

merged 2 commits into from
Nov 16, 2019

Conversation

vgrichina
Copy link
Contributor

Fixes #187

@icerove
Copy link
Contributor

icerove commented Nov 15, 2019

It is really a goo idea to separate the command when you fix it.
Please also update the readme, I think it is not including the new command you added.

}
accountId = accountId || argv.accountId || argv.masterAccount;

if (!accountId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

in this command, accountId is actually not required and user will only follow the command we give not given the account id by default? Maybe change instructions in the readme for this since if not accountId, there will throw an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the intent is to change other tools to return transaction hash as accountId:hash, so we can just have one string that can be copy-pasted

see #187

@vgrichina
Copy link
Contributor Author

It is really a goo idea to separate the command when you fix it.

Seemed like good opportunity to do this. If command was more complex I'd split the change in 2 separate (first move, then fix).

Please also update the readme, I think it is not including the new command you added.

I didn't add any new command, not sure what do you mean.

@icerove
Copy link
Contributor

icerove commented Nov 15, 2019

near generate-key and near repl is not added. Also, still not clear about accountid problem I left above?

@vgrichina
Copy link
Contributor Author

near generate-key and near repl is not added.

these aren't part of this PR

@vgrichina
Copy link
Contributor Author

near generate-key and near repl is not added. Also, still not clear about accountid problem I left above?

1c139dd

@vgrichina vgrichina merged commit 8d414d1 into master Nov 16, 2019
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.

tx-status command must take account id
2 participants