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

hotfix: apply gnokey subcommand flagset #608

Merged
merged 2 commits into from
Mar 21, 2023
Merged

hotfix: apply gnokey subcommand flagset #608

merged 2 commits into from
Mar 21, 2023

Conversation

zivkovicmilos
Copy link
Member

Description

This PR fixes a small bug where flagsets were not being applied for nested gnokey subcommands, namely:

  • gnokey query
  • gnokey broadcast

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)

Checklist (for contributors)

  • I have assigned this PR to myself
  • I have added at least 1 reviewer
  • I have added the relevant labels
  • I have updated the official documentation
  • I have added sufficient documentation in code

Testing

  • I have tested this code with the official test suite
  • I have tested this code manually

Manual tests

Manually made sure the flags for query and broadcast are recognized.

Additional comments

Resolves #607

@zivkovicmilos zivkovicmilos added the hotfix Major bug fix that should be merged ASAP label Mar 16, 2023
@zivkovicmilos zivkovicmilos self-assigned this Mar 16, 2023
@zivkovicmilos zivkovicmilos requested a review from a team as a code owner March 16, 2023 11:06
@zivkovicmilos zivkovicmilos changed the title Hotfix/apply cfg flags hotfix: apply gnokey subcommand flagset Mar 16, 2023
@zivkovicmilos zivkovicmilos mentioned this pull request Mar 16, 2023
Closed
peter7891
peter7891 approved these changes Mar 16, 2023
View reviewed changes
Copy link
Contributor

@peter7891 peter7891 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@zivkovicmilos zivkovicmilos requested a review from moul March 21, 2023 13:50
@moul
Copy link
Member

moul commented Mar 21, 2023

Can you plan to write end-to-end tests to ensure that the basic CLI flows for gnokey and globally still make sense? We already have something similar for gnodev, so let's create something similar for these other applications. Additionally, there is already a CI/CD test using Docker that runs a full chain and interacts with a client.

Feel free to just open an issue if you can't work on this now. Thanks!

@moul moul merged commit 61a6ad9 into gnolang:master Mar 21, 2023
@dongwon8247 dongwon8247 mentioned this pull request Mar 21, 2023
Merged
@zivkovicmilos zivkovicmilos deleted the hotfix/apply-cfg-flags branch March 21, 2023 15:20
@zivkovicmilos
Copy link
Member Author

Can you plan to write end-to-end tests to ensure that the basic CLI flows for gnokey and globally still make sense? We already have something similar for gnodev, so let's create something similar for these other applications. Additionally, there is already a CI/CD test using Docker that runs a full chain and interacts with a client.

Feel free to just open an issue if you can't work on this now. Thanks!

We have unit tests for individual gnokey functionality, but sadly not for the command invocations themselves.

I'll definitely open up a PR that adds extensive coverage to all gnokey command and subcommand invocations, as it's a package we're exporting and people are using it outside of the gno context 💯

r3v4s pushed a commit to r3v4s/gno that referenced this pull request Mar 28, 2023
@zivkovicmilos @r3v4s
hotfix: apply gnokey subcommand flagset (gnolang#608)
75d5dc9
peter7891 pushed a commit that referenced this pull request Mar 29, 2023
@zivkovicmilos @peter7891
hotfix: apply gnokey subcommand flagset (#608)
7774979
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moul moul moul approved these changes

@peter7891 peter7891 peter7891 approved these changes

Assignees

@zivkovicmilos zivkovicmilos

Labels
hotfix Major bug fix that should be merged ASAP
Projects
🚀 The Launch
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

flag provided but not defined: -data
3 participants
@zivkovicmilos @moul @peter7891