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

refactor: use clap as a commands parser #3867

Merged
merged 66 commits into from
Mar 15, 2022

Conversation

therustmonk
Copy link
Contributor

@therustmonk therustmonk commented Feb 22, 2022

Description

The PR refactors replace our own parser with a derived by the clap crate.
It also adds the watch command, and avoids unexpected status printing.

More changes:

  • integration tests use non-interactive mode
  • added --watch parameter to the non-interactive mode to run a command with an interval
  • interactive mode asks for confirmation on exit (to avoid accidental node termination)
  • added the watch command to the interactive mode

Motivation and Context

Improve the user experience of the node's shell.
Simplify maintenance and adding new commands.
Also periodically status prnting breaks the rustyline and prompt chars not printed properly.

How Has This Been Tested?

Manually

Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

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

Looks great! Will wait for you to finish before approving

node_id: UniNodeId,
/// length of time to ban the peer for in seconds
#[clap(default_value_t = std::u64::MAX)]
lenght: u64,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
lenght: u64,
length: u64,

Maybe time_secs is better but up to you

@therustmonk therustmonk force-pushed the clap-commands branch 2 times, most recently from 4f35513 to 0bb2089 Compare March 4, 2022 19:27
@therustmonk therustmonk marked this pull request as ready for review March 5, 2022 13:27
sdbondi
sdbondi previously approved these changes Mar 7, 2022
Copy link
Member

@sdbondi sdbondi left a comment

Choose a reason for hiding this comment

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

Tested a few commands and working well 👍

@sdbondi
Copy link
Member

sdbondi commented Mar 7, 2022

bors r+

bors bot added a commit that referenced this pull request Mar 7, 2022
3867: refactor: use clap as a commands parser r=sdbondi a=DenisKolodin

Description
---
The PR refactors replace our own parser with a derived by the `clap` crate.
It also adds the `watch` command, and avoids unexpected status printing. 

Motivation and Context
---
Improve the user experience of the node's shell.
Simplify maintenance and adding new commands.
Also periodically status prnting breaks the rustyline and prompt chars not printed properly.

How Has This Been Tested?
---
Manually


3892: chore: add license info missing from some crates r=sdbondi a=delta1

Description
---

Adds the BSD-3-Clause license field to our crates that were missing license info



Co-authored-by: Denis Kolodin <deniskolodin@gmail.com>
Co-authored-by: Byron Hambly <bizzle@tari.com>
@bors
Copy link
Contributor

bors bot commented Mar 7, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Mar 7, 2022
Description
---
The PR refactors replace our own parser with a derived by the `clap` crate.
It also adds the `watch` command, and avoids unexpected status printing. 

Motivation and Context
---
Improve the user experience of the node's shell.
Simplify maintenance and adding new commands.
Also periodically status prnting breaks the rustyline and prompt chars not printed properly.

How Has This Been Tested?
---
Manually
@bors
Copy link
Contributor

bors bot commented Mar 7, 2022

Build failed:

@delta1
Copy link
Contributor

delta1 commented Mar 7, 2022

bors retry

bors bot pushed a commit that referenced this pull request Mar 7, 2022
Description
---
The PR refactors replace our own parser with a derived by the `clap` crate.
It also adds the `watch` command, and avoids unexpected status printing. 

Motivation and Context
---
Improve the user experience of the node's shell.
Simplify maintenance and adding new commands.
Also periodically status prnting breaks the rustyline and prompt chars not printed properly.

How Has This Been Tested?
---
Manually
@bors
Copy link
Contributor

bors bot commented Mar 7, 2022

Build failed:

@delta1
Copy link
Contributor

delta1 commented Mar 7, 2022

Seems like something has broken in the integration tests? @deniskolodin

@therustmonk therustmonk force-pushed the clap-commands branch 2 times, most recently from cf88ec1 to b007757 Compare March 12, 2022 15:20
@therustmonk therustmonk force-pushed the clap-commands branch 3 times, most recently from 49dfd64 to 4e06e66 Compare March 13, 2022 19:49
stringhandler
stringhandler previously approved these changes Mar 14, 2022
@stringhandler stringhandler merged commit 3c870ad into tari-project:development Mar 15, 2022
sdbondi added a commit to sdbondi/tari that referenced this pull request Mar 15, 2022
* development: (118 commits)
  chore: clean up providing seed words from LibWallet (tari-project#3906)
  chore: move tari_script into its own crate (tari-project#3909)
  fix(consensus): check blockchain version within valid range (tari-project#3916)
  ci: fix missing npm deps and add javascript ci (tari-project#3910)
  refactor: use clap as a commands parser (tari-project#3867)
  chore: use git tagged tari_utilities and tari-crypto deps (tari-project#3913)
  fix: aligned tables left (tari-project#3899)
  ci: fix vue build
  v0.29.0
  feat!: add recovery byte to output features (tari-project#3727)
  add ffi ci check (tari-project#3915)
  fix(block-sync): use avg latency to determine slow sync peer for block sync (tari-project#3912)
  fix: fix merge mining proxy pool mining (tari-project#3814)
  revert: remove use of blocking tasks for DHT db (reverts tari-project#3887) (tari-project#3901)
  chore: add license info missing from some crates (tari-project#3892)
  fix(core): correctly filter pruned sync peers for block sync (tari-project#3902)
  ci: revert bors squash merge (tari-project#3900)
  fix: update metadata size calculation to use FixedSet.iter()
  docs(rfc): deep links structure convention - deep links is use (tari-project#3897)
  ci: use squash merge for bors (tari-project#3896)
  ...
sdbondi added a commit to Cifko/tari that referenced this pull request Mar 16, 2022
* development:
  chore: clean up providing seed words from LibWallet (tari-project#3906)
  chore: move tari_script into its own crate (tari-project#3909)
  fix(consensus): check blockchain version within valid range (tari-project#3916)
  ci: fix missing npm deps and add javascript ci (tari-project#3910)
  refactor: use clap as a commands parser (tari-project#3867)
  chore: use git tagged tari_utilities and tari-crypto deps (tari-project#3913)
  fix: aligned tables left (tari-project#3899)
  ci: fix vue build
  v0.29.0
  feat!: add recovery byte to output features (tari-project#3727)
  add ffi ci check (tari-project#3915)
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.

4 participants