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

cli: Hides the filler accounts args #34113

Merged

Conversation

brooksprumo
Copy link
Contributor

@brooksprumo brooksprumo commented Nov 16, 2023

Problem

The CLI args for filler accounts are meant for testing/debugging only, and should not be used by regular validators.

(We're also likely going to remove the filler accounts entirely, so removing the CLI args is a good first step.)

Summary of Changes

Hide these args.

@brooksprumo brooksprumo added v1.16 PRs that should be backported to v1.16 v1.17 PRs that should be backported to v1.17 labels Nov 16, 2023
@brooksprumo brooksprumo self-assigned this Nov 16, 2023
@brooksprumo brooksprumo force-pushed the filler-accounts/hide-cli branch from 2d8291c to 6d1ad93 Compare November 16, 2023 16:49
@brooksprumo brooksprumo marked this pull request as ready for review November 16, 2023 17:05
@brooksprumo
Copy link
Contributor Author

Jeff, requesting your review to ensure it's OK to hide these CLI args.

Trent, requesting your review since it touches CLI.

Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

ideally we never would have exposed these to the production binaries in the first place

Copy link
Contributor

@jeffwashington jeffwashington left a comment

Choose a reason for hiding this comment

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

yes, lgtm

Copy link

codecov bot commented Nov 16, 2023

Codecov Report

Merging #34113 (6d1ad93) into master (f005075) will increase coverage by 0.0%.
Report is 24 commits behind head on master.
The diff coverage is 0.0%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #34113   +/-   ##
=======================================
  Coverage    81.9%    81.9%           
=======================================
  Files         812      812           
  Lines      219737   219739    +2     
=======================================
+ Hits       180028   180054   +26     
+ Misses      39709    39685   -24     

@brooksprumo brooksprumo merged commit b4c652e into solana-labs:master Nov 16, 2023
32 checks passed
@brooksprumo brooksprumo deleted the filler-accounts/hide-cli branch November 16, 2023 17:58
mergify bot pushed a commit that referenced this pull request Nov 16, 2023
mergify bot pushed a commit that referenced this pull request Nov 16, 2023
brooksprumo added a commit that referenced this pull request Nov 16, 2023
cli: Hides the filler accounts args (#34113)

(cherry picked from commit b4c652e)

Co-authored-by: Brooks <[email protected]>
brooksprumo added a commit that referenced this pull request Nov 16, 2023
cli: Hides the filler accounts args (#34113)

(cherry picked from commit b4c652e)

Co-authored-by: Brooks <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.16 PRs that should be backported to v1.16 v1.17 PRs that should be backported to v1.17
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants