R4R: Reorganize CLI command structure. #2728

jackzampolin · 2018-11-07T23:05:28Z

Only one cmd.AddCommand is called for each so command registration is more declarative. There are a number of docs fixes that will be entailed here, but we are currently refactoring all of that documentation.

cc @gamarin2 @zramsay @ValarDragon @alessio

codecov · 2018-11-08T00:42:46Z

Codecov Report

Merging #2728 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2728   +/-   ##
========================================
  Coverage    56.63%   56.63%           
========================================
  Files          156      156           
  Lines         9783     9783           
========================================
  Hits          5541     5541           
  Misses        3864     3864           
  Partials       378      378

rigelrozanski

nice reorg!

rigelrozanski · 2018-11-08T02:53:10Z

client/config.go

-	var chainID string
-	chainID, err = types.DefaultChainID()
+	// var chainID string
+	chainID, err := types.DefaultChainID()


rigelrozanski · 2018-11-08T02:54:24Z

client/config.go

@@ -66,8 +68,7 @@ func runConfigCmd(cmd *cobra.Command, args []string) error {
 		Home:      gaiaCLIHome,
 		ChainID:   chainID,
 		TrustNode: trustNode,
-		Encoding:  encoding,
-		Output:    output,
+		Output:    "text",


let's please create and use a newCliConfig(...) function for here (avoid missing structs in the future)

This whole functionality is currently broken. I'll open an issue for that and fix it in another PR if thats OK.

alessio

Agreed with @rigelrozanski's remark on newCliConfig. Other than that, the rest looks good to me. Thanks @jackzampolin

cwgoes

Minor nit and agree with previous comment, but otherwise LGTM.

cwgoes · 2018-11-08T10:29:09Z

client/config.go

-	output := "text"
-	var chainID string
-	chainID, err = types.DefaultChainID()
+	// var chainID string


Let's delete this comment?

alexanderbez · 2018-11-08T13:46:49Z

cmd/gaia/cmd/gaiacli/query.go

+	// Group staking queries under a subcommand
+	stakeQueryCmd := &cobra.Command{
+		Use:   "stake",
+		Short: "Querying commands for staking module",


Suggested change

Short: "Querying commands for staking module",

Short: "Querying commands for the staking module",

alexanderbez · 2018-11-08T13:47:08Z

cmd/gaia/cmd/gaiacli/query.go

+	// Group gov queries under a subcommand
+	govQueryCmd := &cobra.Command{
+		Use:   "gov",
+		Short: "Querying commands for gov module",


Suggested change

Short: "Querying commands for gov module",

Short: "Querying commands for the governance module",

alexanderbez · 2018-11-08T13:47:19Z

cmd/gaia/cmd/gaiacli/query.go

+	// Group slashing queries under a subcommand
+	slashingQueryCmd := &cobra.Command{
+		Use:   "slashing",
+		Short: "Querying commands for slashing module",


Suggested change

Short: "Querying commands for slashing module",

Short: "Querying commands for the slashing module",

alexanderbez · 2018-11-08T13:47:34Z

cmd/gaia/cmd/gaiacli/tx.go

+
+	stakeTxCmd := &cobra.Command{
+		Use:   "stake",
+		Short: "Stake Transactions subcommands",


Suggested change

Short: "Stake Transactions subcommands",

Short: "Staking transaction subcommands",

alexanderbez · 2018-11-08T13:47:54Z

cmd/gaia/cmd/gaiacli/tx.go

+
+	distTxCmd := &cobra.Command{
+		Use:   "dist",
+		Short: "Distribution Transactions subcommands",


Suggested change

Short: "Distribution Transactions subcommands",

Short: "Distribution transaction subcommands",

alexanderbez · 2018-11-08T13:48:04Z

cmd/gaia/cmd/gaiacli/tx.go

+
+	govTxCmd := &cobra.Command{
+		Use:   "gov",
+		Short: "Governance Transactions subcommands",


Suggested change

Short: "Governance Transactions subcommands",

Short: "Governance transaction subcommands",

alexanderbez · 2018-11-08T13:48:16Z

cmd/gaia/cmd/gaiacli/tx.go

+
+	slashingTxCmd := &cobra.Command{
+		Use:   "slashing",
+		Short: "Slashing Transactions subcommands",


Suggested change

Short: "Slashing Transactions subcommands",

Short: "Slashing transaction subcommands",

cmd/gaia/cmd/gaiacli/main.go

zramsay · 2018-11-08T15:24:13Z

awesome, much cleaner :) I've opened #2732 to track the docs update from this PR

Co-Authored-By: jackzampolin <[email protected]>

alexanderbez

utACK 🍍

cwgoes · 2018-11-12T18:55:01Z

Waiting on docs per @jackzampolin

jackzampolin · 2018-11-12T21:18:53Z

Updated new docs to reflect changes here.

alessio · 2018-11-13T07:47:57Z

Tests keep failing

alexanderbez · 2018-11-13T13:54:19Z

@jackzampolin do you need help with this one?

jackzampolin · 2018-11-13T16:28:09Z

@alexanderbez Thanks for the offer, but I think I've got this fixed!

Reorganize CLI command structure. Fixes #2575

d4fb6d4

jackzampolin requested review from cwgoes, ebuchman and rigelrozanski as code owners November 7, 2018 23:05

jackzampolin added 3 commits November 7, 2018 15:35

Fix missing flags issue

c20e383

Address linting issues

cade5f5

Fix gobash CLI testing

0ab5d76

Fix typo

1aae086

rigelrozanski reviewed Nov 8, 2018

View reviewed changes

alessio approved these changes Nov 8, 2018

View reviewed changes

cwgoes approved these changes Nov 8, 2018

View reviewed changes

cwgoes added ready-for-review labels Nov 8, 2018

alexanderbez suggested changes Nov 8, 2018

View reviewed changes

fedekunze reviewed Nov 8, 2018

View reviewed changes

cmd/gaia/cmd/gaiacli/main.go Outdated Show resolved Hide resolved

zramsay mentioned this pull request Nov 8, 2018

Update CLI to match new command structure #2732

Closed

jackzampolin and others added 3 commits November 8, 2018 08:24

Address PR comments

8f4c7df

Update cmd/gaia/cmd/gaiacli/main.go

0ff12bb

Co-Authored-By: jackzampolin <[email protected]>

PENDING

6c261ee

alexanderbez approved these changes Nov 8, 2018

View reviewed changes

jackzampolin mentioned this pull request Nov 9, 2018

Game of Stakes / gaia-9002 release checklist #2754

Closed

27 tasks

Merge branch 'develop' into jack/query-tx-reorg

ac8f2da

jackzampolin added a commit that referenced this pull request Nov 12, 2018

Update docs to reflect changes from #2728

40c0350

Fix failing test

17c119d

zramsay pushed a commit that referenced this pull request Nov 13, 2018

Update docs to reflect changes from #2728

5a671e7

jackzampolin merged commit 12ad39e into develop Nov 13, 2018

jackzampolin deleted the jack/query-tx-reorg branch November 13, 2018 19:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

R4R: Reorganize CLI command structure. #2728

R4R: Reorganize CLI command structure. #2728

jackzampolin commented Nov 7, 2018 •

edited

Loading

codecov bot commented Nov 8, 2018 •

edited

Loading

rigelrozanski left a comment

rigelrozanski Nov 8, 2018

rigelrozanski Nov 8, 2018

jackzampolin Nov 8, 2018

alessio left a comment

cwgoes left a comment

cwgoes Nov 8, 2018

alexanderbez Nov 8, 2018

alexanderbez Nov 8, 2018

alexanderbez Nov 8, 2018

alexanderbez Nov 8, 2018

alexanderbez Nov 8, 2018

alexanderbez Nov 8, 2018

alexanderbez Nov 8, 2018

zramsay commented Nov 8, 2018

alexanderbez left a comment

cwgoes commented Nov 12, 2018

jackzampolin commented Nov 12, 2018

alessio commented Nov 13, 2018

alexanderbez commented Nov 13, 2018

jackzampolin commented Nov 13, 2018

	Short: "Querying commands for staking module",
	Short: "Querying commands for the staking module",

	Short: "Querying commands for gov module",
	Short: "Querying commands for the governance module",

	Short: "Querying commands for slashing module",
	Short: "Querying commands for the slashing module",

	Short: "Stake Transactions subcommands",
	Short: "Staking transaction subcommands",

	Short: "Distribution Transactions subcommands",
	Short: "Distribution transaction subcommands",

	Short: "Governance Transactions subcommands",
	Short: "Governance transaction subcommands",

	Short: "Slashing Transactions subcommands",
	Short: "Slashing transaction subcommands",

R4R: Reorganize CLI command structure. #2728

R4R: Reorganize CLI command structure. #2728

Conversation

jackzampolin commented Nov 7, 2018 • edited Loading

codecov bot commented Nov 8, 2018 • edited Loading

Codecov Report

rigelrozanski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alessio left a comment

Choose a reason for hiding this comment

cwgoes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zramsay commented Nov 8, 2018

alexanderbez left a comment

Choose a reason for hiding this comment

cwgoes commented Nov 12, 2018

jackzampolin commented Nov 12, 2018

alessio commented Nov 13, 2018

alexanderbez commented Nov 13, 2018

jackzampolin commented Nov 13, 2018

jackzampolin commented Nov 7, 2018 •

edited

Loading

codecov bot commented Nov 8, 2018 •

edited

Loading