-
Notifications
You must be signed in to change notification settings - Fork 29
chore: show pubkey_hex at eotsd keys show and list
#320
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
Conversation
dc3728a to
32521de
Compare
709a0c0 to
f902f20
Compare
eotsmanager/cmd/eotsd/daemon/keys.go
Outdated
| } | ||
|
|
||
| listCmd.RunE = runCommandPrintAllKeys | ||
| listCmd.Flags().StringP(flags.FlagOutput, "o", flags.OutputFormatText, "Output format (text|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.
suggestion to remove this all good to use --output json
| listCmd.Flags().StringP(flags.FlagOutput, "o", flags.OutputFormatText, "Output format (text|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.
Hmm when I remove this though, the test complains when using --output
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.
it shouldn't since it works locally, maybe there is something weird with the test setup
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.
babylon_e2e_test.go:407:
Error Trace: /home/runner/work/finality-provider/finality-provider/itest/babylon/babylon_e2e_test.go:407
Error: Received unexpected error:
flag accessed but not defined: output
Test: TestPrintEotsCmd
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.
from the CI
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.
humm, I had removed and run the test and it passes TestPrintEotsCmd
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.
done at #341
RafilxTenfen
left a comment
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 🎉
| panic("failed to find keys list command") | ||
| } | ||
|
|
||
| listCmd.RunE = runCommandPrintAllKeys |
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.
we could also remove CommandPrintAllKeys since now there is a keys ls subcommand
What do you think @Lazar955 ?
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.
yep makes sense
f8ae638 to
09ce71b
Compare
| panic("failed to find keys list command") | ||
| } | ||
|
|
||
| listCmd.RunE = runCommandPrintAllKeys |
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.
yep makes sense
closes: #274 When using the CLI the user can now retrieve their hex from show and list commands. Added in custom methods to replace the default SDK ones --------- Co-authored-by: RafilxTenfen <rafaeltenfen.rt@gmail.com>
closes: #274
When using the CLI the user can now retrieve their hex from show and list commands. Added in custom methods to replace the default SDK ones