Skip to content

refactor: rename server fields in config#833

Merged
lumtis merged 7 commits intodevelopfrom
refacto/rename-config-server
Mar 11, 2021
Merged

refactor: rename server fields in config#833
lumtis merged 7 commits intodevelopfrom
refacto/rename-config-server

Conversation

@lumtis
Copy link
Copy Markdown
Contributor

@lumtis lumtis commented Mar 10, 2021

No description provided.

@lumtis lumtis linked an issue Mar 10, 2021 that may be closed by this pull request
@fadeev
Copy link
Copy Markdown
Contributor

fadeev commented Mar 10, 2021

I know you know, but just making sure we don't change faucet.port 🙏

@ilgooz
Copy link
Copy Markdown
Contributor

ilgooz commented Mar 10, 2021

Can we have host while keeping port and undocument port now so, we can drop it at some point?

@lumtis lumtis marked this pull request as ready for review March 10, 2021 16:50
@lumtis lumtis requested review from fadeev and ilgooz as code owners March 10, 2021 16:50
fadeev
fadeev previously approved these changes Mar 11, 2021
Copy link
Copy Markdown
Contributor

@fadeev fadeev left a comment

Choose a reason for hiding this comment

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

👏


host := conf.Faucet.Host
if conf.Faucet.Port != 0 {
host = fmt.Sprintf("0.0.0.0:%d", conf.Faucet.Port)
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.

Suggested change
host = fmt.Sprintf("0.0.0.0:%d", conf.Faucet.Port)
host = fmt.Sprintf(":%d", conf.Faucet.Port)

Let's follow the same pattern here:
https://github.com/tendermint/starport/blob/e94b428a1c4856cf101463d9afc8d8d1cec632a8/starport/chainconf/config.go#L24-L30

fmt.Fprintf(c.stdLog(logStarport).out, "🌍 Running a Cosmos '%[1]v' app with Tendermint at %s.\n", c.app.Name, xurl.HTTP(conf.Host.RPC))
fmt.Fprintf(c.stdLog(logStarport).out, "🌍 Running a server at %s (LCD)\n", xurl.HTTP(conf.Host.API))

host := conf.Faucet.Host
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.

Can we move this part related to determining address for the faucet to starporconf.FaucetHost(), and reuse it here?

return err
}

host := conf.Faucet.Host
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.

Can we move this part related to determining address for the faucet to starporconf.FaucetHost(), and reuse it here?

@lumtis lumtis merged commit 7b9f4c4 into develop Mar 11, 2021
@lumtis lumtis deleted the refacto/rename-config-server branch March 11, 2021 09:55
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 have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: rename servers to host top-level prop

3 participants