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.

Sign up for GitHub

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

Jump to bottom

CLI Refactor #6571

Closed
22 of 27 tasks
alexanderbez opened this issue Jul 1, 2020 · 3 comments
Closed
22 of 27 tasks

CLI Refactor #6571

alexanderbez opened this issue Jul 1, 2020 · 3 comments
Assignees
@alexanderbez
Labels
C:CLI
Milestone
v0.40 [Stargate]

Comments

@alexanderbez
Copy link
Contributor

alexanderbez commented Jul 1, 2020 •
edited
Loading

Summary

The current state of the CLI in the SDK is a bit of a mess, with the use of Viper global/singleton, ad-hoc flag instantiation and binding, and magically instantiation of client.Context. It makes testing difficult and runtime errors hard to debug.

Proposal

Once #6423 is finalized and merged, we'll have the ability to remove a lot of the existing cruft from the CLI framework. Namely, we should:

Stretch Goal:

  • Support --config flag where if provided, any field found will set/override any flag/arg.

/cc @aaronc @anilcse @sahith-narahari @alessio

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez alexanderbez added this to the v0.39 milestone Jul 1, 2020
@alexanderbez alexanderbez self-assigned this Jul 1, 2020
@alexanderbez alexanderbez mentioned this issue Jul 1, 2020
Merged
9 tasks
@colin-axner
Copy link
Contributor

Since this is a quite a big change. Could we add creating a doc to the to do list for this. Maybe as an upgrade doc or explaining the scope of the changes for projects dependent upon this area of code.

@alexanderbez
Copy link
Contributor Author

@colin-axner sure, but the only major differences from a UX POV are:

  1. the removal of config.toml
  2. a single binary
  3. in process API

We'll have an upgrading doc that will cover all of this stuff.

This was referenced Jul 3, 2020
Merged
Merged
Merged
Merged
@aaronc aaronc mentioned this issue Jul 8, 2020
Closed
43 tasks
@alexanderbez alexanderbez mentioned this issue Jul 8, 2020
Merged
9 tasks
@amaury1093 amaury1093 mentioned this issue Jul 9, 2020
Merged
20 tasks
This was referenced Jul 20, 2020
Merged
Merged
@ethanfrey
Copy link
Contributor

Great job bez!

Looking forward to the new cli

@ethanfrey ethanfrey mentioned this issue Jul 21, 2020
Merged
9 tasks
@sahith-narahari sahith-narahari mentioned this issue Aug 7, 2020
Merged
9 tasks
alessio pushed a commit that referenced this issue Aug 17, 2020
@sahith-narahari @aaronc
Add cli test for query Account (#6973)
77124da 
Closes: #7007

Ref: #6571

Co-authored-by: Aaron Craelius <[email protected]>
Co-authored-by: Alessio Treglia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alexanderbez alexanderbez

Labels
C:CLI
Projects
None yet
Milestone
v0.40 [Stargate]
Development

No branches or pull requests
3 participants
@alexanderbez @ethanfrey @colin-axner