8000 refactor: package for blockchain application commands by lumtis · Pull Request #583 · ignite/cli · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

refactor: package for blockchain application commands #583

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

Merged
merged 20 commits into from
Jan 4, 2021

Conversation

lumtis
Copy link
Contributor
@lumtis lumtis commented Dec 21, 2020

Defines in the package chaincmd all the commands related to a blockchain. The functions return a step.Option to run the command with cmdrunner.
You can define the global flags like home or keyring-backend when creating the ChainCmd object. So that it would be easy to implement a global home that will be used by Starport for all commands.

Lot of Launchpad commands have the same format as the Stargate commands. We have the specific Launchpad command in the same object with the prefix Launchpad this choice can be discussed.

@lumtis lumtis linked an issue Dec 21, 2020 that may be closed by this pull request
@lumtis lumtis marked this pull request as ready for review December 22, 2020 09:57
@lumtis lumtis requested review fro 8000 m fadeev and ilgooz as code owners December 22, 2020 09:57
ilgooz
ilgooz previously approved these changes Dec 29, 2020
Copy link
Member
@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

I really like this package, nice work! 🍷

@ilgooz ilgooz added this to the Refactoring milestone Jan 4, 2021

// GentxCommand returns the command to generate a gentx for the chain
func (c ChainCmd) GentxCommand(
validatorName string,
Copy link
Member
@ilgooz ilgooz Jan 4, 2021

Choose a reason for hiding this comment

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

I think it would cleaner if we don't require optional flags as arguments. Instead, maybe we can use a struct to pass these or ...GenTxOptions.

}

// ImportKeyCommand returns the command to import a key into the chain keyring from a mnemonic
func (c ChainCmd) ImportKeyCommand(accountName string) step.Option {
Copy link
Member

Choose a reason for hiding this comment

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

Since --recover option is interactive, what do you think about implementing ImportKeyCommand like this by using step.Write to write interactive input?

But this will require ImportKeyCommand to return a []step.Option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer to keep minimal usage for this command and only returning step.Option of the command

@lumtis lumtis requested a review from ilgooz January 4, 2021 13:01
Copy link
Member
@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

Perfect! 👌 ~tested partially.

@lumtis lumtis merged commit 8baa8e5 into develop Jan 4, 2021
@lumtis lumtis deleted the refactors/chain-commands branch January 4, 2021 13:33
faddat added a commit to faddat/starport that referenced this pull request Jan 7, 2021
# By vms (85) and others
# Via GitHub (23) and others
* https://github.com/tendermint/starport: (276 commits)
  refactor: use a fixed-name dir for Starport Network chain sources (ignite#597)
  Update devices.yml (ignite#616)
  refactor: set chaincmd commands as version agnostic (ignite#614)
  feature(network): show errors from appd binary (ignite#506)
  refactor: use stable version of Starport in docs (ignite#610)
  refactor: package for blockchain application commands (ignite#583)
  refactor: cleanup home files (ignite#595)
  feat(pkg/gomodulepath): add tests for paths with versions
  docs: install.md (ignite#602)
  fix: semantic-pr
  fix: determining version number in go paths (ignite#598)
  docs: update readme with a link to v0.13 video (ignite#593)
  diable snapcraft
  ap get update
  fix: changelog small fix (ignite#588)
  Update changelog.md (ignite#587)
  feat(network): disable create command for Launchpad apps (ignite#586)
  refactor(docs): change 'pulling blockchain' to 'fetching source code' (ignite#585)
  Update version of tendermint/vue (ignite#582)
  fix: integration tests (ignite#581)
  ...

# Conflicts:
#	.github/workflows/pi-image.yml
#	.pi/starport.json
#	go.mod
#	go.sum
#	starport/templates/app/launchpad/.github/workflows/build.yml.plush
#	starport/templates/app/launchpad/.github/workflows/pi.yml.plush
#	starport/templates/app/launchpad/.pi/image.bash.plush
#	starport/templates/app/launchpad/.pi/pibuild.json.plush
#	starport/templates/app/launchpad/.pi/{{appName}}d.service.plush
#	starport/ui/package-lock.json
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already hav 6841 e an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor: centralize commands of sdk CLI in a package
2 participants
0