-
Notifications
You must be signed in to change notification settings - Fork 651
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
Fix bbolt keys
and bbolt get
to prevent them from panicking when no parameter provided
#682
Conversation
@ahrtr i will go through all the cmds now just this is to get early review |
efd618e
to
7ac779a
Compare
only |
7ac779a
to
a181184
Compare
Just as we talked in the meeting, the fix overall looks good. The only concern is on the output, which mixes the error message and the command usage. We should only need to take care of the error message, and let the command generic utility to take care of the usage. Output of this PR
Good example of other commands
|
a181184
to
b2619f4
Compare
@ahrtr fixed, now the output
|
cmd/bbolt/main.go
Outdated
err := fmt.Errorf("Error: %w.\n", ErrNotEnoughArgs) | ||
fmt.Fprintln(cmd.Stderr, cmd.Usage()) | ||
return err |
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.
err := fmt.Errorf("Error: %w.\n", ErrNotEnoughArgs) | |
fmt.Fprintln(cmd.Stderr, cmd.Usage()) | |
return err | |
return ErrNotEnoughArgs |
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, but this way the usage
will not be printed
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.
Your previous implement (see below) isn't consistent with the cobra style command's output.
Overall it isn't a big deal for now. Let's refactor it in future.
$ ./bbolt keys
usage: bolt keys PATH [BUCKET...]
Print a list of keys in the given (sub)bucket.
=======
Additional options include:
--format
Output format. One of: auto|ascii-encoded|hex|bytes|redacted (default=auto)
Print a list of keys in the given bucket.
Error: not enough arguments.
cmd/bbolt/main.go
Outdated
err := fmt.Errorf("Error: %w.\n", ErrNotEnoughArgs) | ||
fmt.Fprintln(cmd.Stderr, cmd.Usage()) | ||
return err |
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.
err := fmt.Errorf("Error: %w.\n", ErrNotEnoughArgs) | |
fmt.Fprintln(cmd.Stderr, cmd.Usage()) | |
return err | |
return ErrNotEnoughArgs |
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.
ditto
There are some inconsistency between the legacy commands and cobra style commands, we will need to do some refactor in future after we cut release-1.4 branch. |
sure, i will do the cobra migration as we discussed after the release 👍🏽 |
Signed-off-by: Mustafa Elbehery <melbeher@redhat.com>
b2619f4
to
3405ebb
Compare
bbolt keys
and bbolt get
to prevent them from panicking when no parameter provided
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.
lgtm
Thanks
resolves #681
cc @ahrtr