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

feat: add the filter mode to Cli #435

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

chungquantin
Copy link
Collaborator

Add the new method filter_mode() to the Cli struct that enables the filter mode to the Select and MultiSelect. We need this feature for the #420

The implementation PR of the filter mode in cliclack that includes images of how the feature looks like.

@chungquantin chungquantin requested a review from AlexD10S March 4, 2025 04:22
@chungquantin chungquantin changed the title feat: add the filter mode to cli feat: add the filter mode to Cli Mar 4, 2025
@chungquantin chungquantin added the ready-for-final-review The PR is ready for final review label Mar 4, 2025
Copy link

codecov bot commented Mar 4, 2025

Codecov Report

Attention: Patch coverage is 61.53846% with 25 lines in your changes missing coverage. Please review.

Project coverage is 75.28%. Comparing base (9d82d84) to head (cbc84e0).

Files with missing lines Patch % Lines
crates/pop-cli/src/cli.rs 45.65% 24 Missing and 1 partial ⚠️
@@                          Coverage Diff                          @@
##           chungquantin/chore-deny_action_v2     #435      +/-   ##
=====================================================================
- Coverage                              75.46%   75.28%   -0.18%     
=====================================================================
  Files                                     64       64              
  Lines                                  14108    14029      -79     
  Branches                               14108    14029      -79     
=====================================================================
- Hits                                   10646    10562      -84     
- Misses                                  2100     2120      +20     
+ Partials                                1362     1347      -15     
Files with missing lines Coverage Δ
crates/pop-cli/src/commands/build/spec.rs 74.08% <100.00%> (+0.27%) ⬆️
crates/pop-cli/src/commands/call/chain.rs 74.76% <100.00%> (+0.11%) ⬆️
crates/pop-cli/src/commands/call/contract.rs 80.38% <100.00%> (+0.07%) ⬆️
crates/pop-cli/src/commands/clean.rs 83.33% <100.00%> (+0.25%) ⬆️
crates/pop-cli/src/cli.rs 63.00% <45.65%> (-2.47%) ⬇️

... and 4 files with indirect coverage changes

@AlexD10S
Copy link
Collaborator

AlexD10S commented Mar 4, 2025

CI is failing in deny and docker.

I see the deny error, which was an open issue yesterday: EmbarkStudios/cargo-deny-action#91
I believe we need to update it to v2, as seen in this commit: restatedev/restate@b16d7da

Also I see some clippy warnings.

@chungquantin
Copy link
Collaborator Author

CI is failing in deny and docker.

I see the deny error it was an open issue yesterday: EmbarkStudios/cargo-deny-action#91 I believe we only need to update it to v2: restatedev/restate@b16d7da

Also I see some clippy warnings.

For the CI, let me update the #431 to include that error as well and we merge that first. I would love to resolve an issue with the CI asap.

For the clippy warnings, because we don't use those methods anywhere yet. They are expected to be used in the interactive UI of pallet benchmarking PR

@chungquantin chungquantin changed the base branch from main to chungquantin/chore-deny_action_v2 March 4, 2025 15:40
Base automatically changed from chungquantin/chore-deny_action_v2 to main March 4, 2025 21:06
Copy link
Collaborator

@AlexD10S AlexD10S left a comment

Choose a reason for hiding this comment

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

Looks good!
Just a nitpick, before merging fix a warning message, with the zombinet-sdk upgrade

warning: use of deprecated method `pop_parachains::NetworkNode::client`: Use `wait_client` instead.
   --> crates/pop-cli/src/commands/up/network.rs:174:61
    |
174 | ...                   let relay_endpoint = network.relaychain().nodes()[0].client().await?;

@chungquantin chungquantin force-pushed the chungquantin/feat-filter_mode branch from 9990d43 to 981b5f4 Compare March 5, 2025 01:56
@chungquantin chungquantin force-pushed the chungquantin/feat-filter_mode branch from 981b5f4 to e470707 Compare March 5, 2025 01:58
@chungquantin chungquantin force-pushed the chungquantin/feat-filter_mode branch from e470707 to 33b370b Compare March 5, 2025 02:00
@chungquantin chungquantin self-assigned this Mar 5, 2025
@chungquantin chungquantin requested a review from AlexD10S March 5, 2025 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-final-review The PR is ready for final review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants