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

List the default value for --accounts in CLI help #35254

Merged
merged 2 commits into from
Feb 20, 2024

Conversation

steviez
Copy link
Contributor

@steviez steviez commented Feb 20, 2024

Problem

--accounts is not a required argument, so list what the default location is if unspecified. We do this for several others similar arguments.

validator/src/cli.rs Outdated Show resolved Hide resolved
@steviez steviez requested a review from brooksprumo February 20, 2024 16:53
brooksprumo
brooksprumo previously approved these changes Feb 20, 2024
Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

:shipit:

validator/src/cli.rs Outdated Show resolved Hide resolved
@brooksprumo
Copy link
Contributor

Wanna fix this issue in ledger-tool too?

@steviez
Copy link
Contributor Author

steviez commented Feb 20, 2024

Wanna fix this issue in ledger-tool too?

Yes, how could I forget about ledger-tool 😆

Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

Lgtm. Can you update the PR's title too? Maybe just remove the word "validator'?

@steviez steviez changed the title List the default accounts location in validator CLI help List the default value for --accounts in CLI help Feb 20, 2024
Copy link

codecov bot commented Feb 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e406402) 81.6% compared to head (882222d) 81.6%.
Report is 19 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #35254   +/-   ##
=======================================
  Coverage    81.6%    81.6%           
=======================================
  Files         833      833           
  Lines      224760   224760           
=======================================
+ Hits       183452   183486   +34     
+ Misses      41308    41274   -34     

@steviez steviez merged commit a1c39a3 into solana-labs:master Feb 20, 2024
35 checks passed
@steviez steviez deleted the val_accounts_default branch February 20, 2024 19:12
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.

2 participants