Skip to content

Conversation

@lumtis
Copy link
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
Contributor

fadeev commented Mar 10, 2021

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

@ilgooz
Copy link
Member

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
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
Member

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
Member

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
Member

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

4 participants