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

Add solana_clap_utils::hidden_unless_forced() #30843

Merged
merged 2 commits into from
Mar 22, 2023

Conversation

ryoqun
Copy link
Member

@ryoqun ryoqun commented Mar 22, 2023

Problem

There's no nice way to forcibly expose all hidden args of CLI. It's desirable to see all sometimes for dev purpose.

Summary of Changes

Sneakily introduce such a mechanism

context: spin off from https://github.com/solana-labs/solana/pull/30746/files#r1140313895

@ryoqun ryoqun marked this pull request as ready for review March 22, 2023 07:11
@ryoqun ryoqun requested a review from yihau March 22, 2023 07:15
clap-utils/src/lib.rs Outdated Show resolved Hide resolved
mvines
mvines previously approved these changes Mar 22, 2023
Co-authored-by: mvines <mvines@gmail.com>
@ryoqun
Copy link
Member Author

ryoqun commented Mar 22, 2023

@mvines thanks a lot. :)

@ryoqun ryoqun added the automerge Merge this Pull Request automatically once CI passes label Mar 22, 2023
@ryoqun ryoqun removed the request for review from yihau March 22, 2023 07:23
@codecov
Copy link

codecov bot commented Mar 22, 2023

Codecov Report

Merging #30843 (19e2ac8) into master (ac8c31b) will decrease coverage by 0.1%.
The diff coverage is 20.6%.

@@            Coverage Diff            @@
##           master   #30843     +/-   ##
=========================================
- Coverage    81.5%    81.5%   -0.1%     
=========================================
  Files         723      723             
  Lines      203473   203577    +104     
=========================================
+ Hits       165922   165935     +13     
- Misses      37551    37642     +91     

@mergify mergify bot merged commit 721719d into solana-labs:master Mar 22, 2023
@CriesofCarrots
Copy link
Contributor

Seems unlikely that most engs will know or remember to do this when adding a new hidden() tag, esp since most don't see all PR notifications like I do. But perhaps you are planning to audit every so often?

@ryoqun
Copy link
Member Author

ryoqun commented Mar 22, 2023

Seems unlikely that most engs will know or remember to do this when adding a new hidden() tag, esp since most don't see all PR notifications like I do. But perhaps you are planning to audit every so often?

yeah, i'll do manual audit for any in-flight .hidden(true) if any in open prs, like after 1 month? Then, I'm planning to eventually add a single git grep to sanity.sh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants