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

confirmation_height_clear cli account param #3836

Conversation

JerzyStanislawski
Copy link
Contributor

@JerzyStanislawski JerzyStanislawski commented Jun 9, 2022

Make account argument required for confirmation_height_clear cli command.
Issue reference: #3831
resolves #3831

@thsfs thsfs requested review from thsfs and dsiganos June 23, 2022 19:30
thsfs
thsfs previously approved these changes Jun 23, 2022
@thsfs
Copy link
Contributor

thsfs commented Jun 23, 2022

Sent a fix to the line I mentioned earlier, there is also the password option for account_create, not related to confirmation_height_clear, but changed in the first commit.

@thsfs
Copy link
Contributor

thsfs commented Jun 23, 2022

@dsiganos , could you double check and get this merged if you approve?

@thsfs thsfs added this to the V24.0 milestone Jun 23, 2022
@thsfs thsfs added documentation This item indicates the need for or supplies updated or expanded documentation cli Changes related to the Command Line Interface labels Jun 23, 2022
auto transaction (node.node->store.tx_begin_write ());
reset_confirmation_heights (transaction, node.node->network_params.ledger, node.node->store);
std::cout << "Confirmation heights of all accounts (except genesis which is set to 1) are set to 0" << std::endl;
std::cerr << "confirmation_height_clear command requires one <account> option\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

It could also say: "or 'any' to clear all accounts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Improved this and also the help message to the --confirmation_height_clear option.

dsiganos
dsiganos previously approved these changes Jun 23, 2022
- Improve the error message to say the account can be 'all'
- Improve the confirmation_height_clear help message to inform the value
'all' can be passed to clear all accounts
@thsfs thsfs dismissed stale reviews from dsiganos and themself via dda9593 June 23, 2022 22:12
@thsfs thsfs requested a review from dsiganos June 23, 2022 22:13
@dsiganos dsiganos merged commit bee7044 into nanocurrency:develop Jun 24, 2022
@thsfs
Copy link
Contributor

thsfs commented Jun 24, 2022

Thanks @JerzyStanislawski !

@JerzyStanislawski JerzyStanislawski deleted the confirmation_height_clear-cli-param branch June 24, 2022 21:16
gr0vity-dev pushed a commit to gr0vity-dev/nano-node that referenced this pull request Jul 9, 2022
* confirmation_height_clear cli account param

* Add back missing password option

* Improve the error/help messages to the required account option

- Improve the error message to say the account can be 'all'
- Improve the confirmation_height_clear help message to inform the value
'all' can be passed to clear all accounts

Co-authored-by: Thiago Silva <thiago@nano.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Changes related to the Command Line Interface documentation This item indicates the need for or supplies updated or expanded documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change the confirmation_height_clear option to require the account argument
3 participants