-
Notifications
You must be signed in to change notification settings - Fork 402
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
Implement PinCode and UnpinCode proposal client handlers #707
Conversation
Codecov Report
@@ Coverage Diff @@
## master #707 +/- ##
==========================================
- Coverage 60.21% 59.24% -0.98%
==========================================
Files 48 48
Lines 5361 5449 +88
==========================================
Hits 3228 3228
- Misses 1903 1991 +88
Partials 230 230
|
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.
Changes look good 👍
I have not had a chance to test the commands manually but will do tomorrow
@@ -14,4 +14,6 @@ var ProposalHandlers = []govclient.ProposalHandler{ | |||
govclient.NewProposalHandler(cli.ProposalMigrateContractCmd, rest.MigrateProposalHandler), | |||
govclient.NewProposalHandler(cli.ProposalUpdateContractAdminCmd, rest.UpdateContractAdminProposalHandler), | |||
govclient.NewProposalHandler(cli.ProposalClearContractAdminCmd, rest.ClearContractAdminProposalHandler), | |||
govclient.NewProposalHandler(cli.ProposalPinCodesCmd, rest.PinCodeProposalHandler), | |||
govclient.NewProposalHandler(cli.ProposalUnpinCodesCmd, rest.UnpinCodeProposalHandler), |
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.
👍
cmd.Flags().String(cli.FlagDeposit, "", "Deposit of proposal") | ||
cmd.Flags().String(cli.FlagProposal, "", "Proposal file path (if this path is given, other proposal flags are ignored)") | ||
// type values must match the "ProposalHandler" "routes" in cli | ||
cmd.Flags().String(flagProposalType, "", "Permission of proposal, types: store-code/instantiate/migrate/update-admin/clear-admin/text/parameter_change/software_upgrade") |
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.
Not scope of this PR: this flag looks odd. It should be "Type" instead of "Permission" here and contain all possible types. I am unsure if it is still needed
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.
Good PR but 3 fixes required to make it work. I missed them when I 👁️ at it the first time.
Co-authored-by: Alexander Peters <[email protected]>
Thanks 💐 |
No description provided.