Skip to content

fix: init with config chain-id#9762

Merged
mergify[bot] merged 4 commits into
masterfrom
ryan/fix-init-chain-id
Jul 26, 2021
Merged

fix: init with config chain-id#9762
mergify[bot] merged 4 commits into
masterfrom
ryan/fix-init-chain-id

Conversation

@ryanchristo
Copy link
Copy Markdown
Contributor

@ryanchristo ryanchristo commented Jul 24, 2021

Description

Closes: #9604, Ref: #9644

The init command uses the chain-id from the client config if --chain-id is not provided.


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules -n/a
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code -n/a
  • updated the relevant documentation or specification -n/a
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@github-actions github-actions Bot added C:CLI C:x/genutil genutil module issues labels Jul 24, 2021
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 24, 2021

Codecov Report

Merging #9762 (2aa8ef0) into master (b34ceec) will decrease coverage by 0.15%.
The diff coverage is 80.00%.

❗ Current head 2aa8ef0 differs from pull request most recent head d347c38. Consider uploading reports for the commit d347c38 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9762      +/-   ##
==========================================
- Coverage   63.47%   63.31%   -0.16%     
==========================================
  Files         566      563       -3     
  Lines       52794    52729      -65     
==========================================
- Hits        33513    33388     -125     
- Misses      17378    17435      +57     
- Partials     1903     1906       +3     
Impacted Files Coverage Δ
x/genutil/client/cli/init.go 67.39% <80.00%> (-0.03%) ⬇️
types/dec_coin.go 77.27% <0.00%> (-21.93%) ⬇️
x/bank/keeper/send.go 84.02% <0.00%> (-1.62%) ⬇️
x/auth/tx/decoder.go 79.66% <0.00%> (-0.18%) ⬇️
x/bank/keeper/view.go 88.33% <0.00%> (-0.10%) ⬇️
x/staking/client/testutil/suite.go 99.42% <0.00%> (-0.01%) ⬇️
x/auth/client/testutil/suite.go 95.91% <0.00%> (ø)
x/bank/migrations/v044/store.go
x/bank/migrations/v044/keys.go
x/bank/migrations/v043/keys.go
... and 4 more

@ryanchristo ryanchristo force-pushed the ryan/fix-init-chain-id branch from 3535b9a to 41d3039 Compare July 24, 2021 00:40
@orijbot
Copy link
Copy Markdown

orijbot commented Jul 24, 2021

Visit https://dashboard.github.orijtech.com?pr=9762&repo=cosmos%2Fcosmos-sdk to see benchmark details.

@ryanchristo ryanchristo marked this pull request as ready for review July 26, 2021 15:03
Comment thread x/genutil/client/cli/init.go Outdated
Comment on lines +82 to +88
chainID, _ := cmd.Flags().GetString(flags.FlagChainID)
if chainID == "" {
chainID = fmt.Sprintf("test-chain-%v", tmrand.Str(6))
if clientCtx.ChainID == "" {
chainID = fmt.Sprintf("test-chain-%v", tmrand.Str(6))
} else {
chainID = clientCtx.ChainID
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: this could be cleaned up with just a single switch clause.

switch {
  case chainID != "":
  case clientCtx.ChainID != ""
  default:
}

Copy link
Copy Markdown
Member

@technicallyty technicallyty 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!

@mergify mergify Bot merged commit f4d0682 into master Jul 26, 2021
@mergify mergify Bot deleted the ryan/fix-init-chain-id branch July 26, 2021 22:52
@amaury1093 amaury1093 mentioned this pull request May 23, 2022
72 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C:CLI C:x/genutil genutil module issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

init command should fetch chain id from config

4 participants