-
Notifications
You must be signed in to change notification settings - Fork 563
refactor: set chaincmd commands as version agnostic #614
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
Conversation
starport/pkg/chaincmd/chaincmd.go
Outdated
return step.Exec(c.appCmd, c.attachHome(command)...) | ||
|
||
// Check version | ||
if c.isStargate() { |
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.
How about simplifying this and other similar, like below:
return step.Exec(c.daemonCmd(), c.attachCLIHome(command)...)
// where daemonCmd is:
func (c ChainCmd) daemonCmd() string {
if c.isStargate() {
return c.appCmd
}
return c.cliCmd
}
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.
c.attachHome(command)
is changed to c.attachCLIHome(command)
as well so we should have
func (c ChainCmd) daemonCmd(command) string {
if c.isStargate() {
return step.Exec(c.appCmd, c.attachHome(command)...)
}
returnstep.Exec(c. cliCmd, c. attachCLIHome(command)...)
}
The name daemonCmd
is a bit confusing because Launchpad daemon is also appd
I would rather call it cliCmd
in the sense that for launchpad cli is explicitly appcli
and the cli for Stargate is appd
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.
🎊
starport/pkg/chaincmd/chaincmd.go
Outdated
@@ -270,6 +275,24 @@ func (c ChainCmd) ShowNodeIDCommand() step.Option { | |||
return step.Exec(c.appCmd, c.attachHome(command)...) | |||
} | |||
|
|||
// SetConfigCommand returns the command to set config value |
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.
// SetConfigCommand returns the command to set config value | |
// LaunchpadSetConfigCommand returns the command to set config value |
starport/pkg/chaincmd/chaincmd.go
Outdated
return c.launchpadSetConfigCommand(name, value) | ||
} | ||
|
||
// RestServerCommand returns the command to start the CLI REST server |
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.
// RestServerCommand returns the command to start the CLI REST server | |
// LaunchpadRestServerCommand returns the command to start the CLI REST server |
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.
🍷
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.
utACK 🙏
# 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
The necessary version is automatically used if
WithLaunchpad
command is furnished or not.All commands become version agnostic.
We fix in this PR an error: home directory was the same for appd and appcli commands while it must be different.