-
Notifications
You must be signed in to change notification settings - Fork 73
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
[3.2] Build framework for leap-util #150
Conversation
With both cleos & leap-util using CLI11, does it make sense to have just one instance of it? Does it make sense to have that one instance be a submodule from upstream? |
Yes! This is the plan. And cleos will get autocompletion and --help-all for free. |
// Originally designed by Henry Schreiner | ||
// https://github.com/CLIUtils/CLI11 | ||
// | ||
// This is a standalone header file generated by MakeSingleHeader.py in CLI11/scripts | ||
// from: v1.9.0 | ||
// from: 4c832a2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not the same version as in AntelopeIO/CLI11's main
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, thank you! It's 1d3b0a7 now.
If I just run |
I know we were just aiming for 1:1 of current functionality, but there are some missed opportunities to improve the commands to make operator lifes' easier. Just based on a cursory look two things stood out: build info
For example, I could image an operator trying to do something like
(the above assumes some canonical string representation of the environment, which might be kinda bad, and doesn't capture that in addtion to environment there could be other reasons the state isn't compatible). Maybe instead, just as an off the cuff thought, there would be a
block log queryThe |
when I just run
That alone is not what I'd expect. And then if I run it again...
The tool made a new |
sub->add_subcommand("to-json", "convert snapshot file to convert to json format"); | ||
|
||
// options | ||
sub->add_option("--input-file,-i", opt->input_file, "snapshot file to convert to json format, writes to <file>.json if output file not specified (tmp state dir used), and exit.")->required(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite follow what this description of the option is trying to tell me. It seems to be saying if I give an input file without an output file, the output file will be basenameofinput.json; but that's in conflict of the output-file
description which says no output file means output goes to stdout. And I don't know what "tmp state dir" means in this context.
More generally, since the options are attached to snapshot
, not to-json
, the descriptions are probably too narrowly defined solely for the to-json
use case. There are a number of useful operations on a snapshot file we can add in the future that don't have anything to do with json.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocklog needs more refactoring like specific options for a subcommand, ... @766C6164 and talked about them.
I wonder if |
This is initial proposed implementation of leap-util utility comprising following features:
This PR still work in progress and things will change, notes about changes will be added in comments.