Add dockerfile and version+help commands#37
Conversation
gballet
left a comment
gballet
left a comment
There was a problem hiding this comment.
Nice. I have a few questions regarding your changes to the management of the --help option. I'd rather have this using the structure instead of manually parsing the arguments, but I see that this is broken as well. I'd rather fix that upstream.
pkgs/cli/src/main.zig
Outdated
| pub const __messages__ = .{ | ||
| .genesis = "genesis time", | ||
| .genesis = "Genesis time for the chain (default: 1234)", | ||
| .num_validators = "Number of validators (default: 4)", |
There was a problem hiding this comment.
if you define your own help message, this change won't be necessary afaict
There was a problem hiding this comment.
Is the reason you are manually handling the help message, that you wanted to display the list of commands? I see that this is indeed not working with a call to printHelp(), and also that it does a pretty shitty job of displaying the description. Is that an urgent problem to solve? if not, I'd rather fix it in the imported module.
pkgs/cli/src/main.zig
Outdated
| _ = try stdout.write("Version: "); | ||
| _ = try stdout.write(app_version); |
There was a problem hiding this comment.
is it necessary for your to write the version here? We could add a --version option instead.
Signed-off-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Signed-off-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
gballet
left a comment
gballet
left a comment
There was a problem hiding this comment.
Thanks for taking care of this! I had to rummage a bit in the zigcli source code to find out what was going wrong with just defining a help field on the structure. It's now fixed and so I'd rather use the structure as it's what it's been designed for. If you think there is a good reason not to do that, we can reopen a PR and use your "manual" code.
gballet
changed the title
2 participants
No description provided.