Skip to content
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

Tweak server start-up text #624

Merged
merged 4 commits into from
Jul 15, 2024

Conversation

dandavison
Copy link
Contributor

@dandavison dandavison commented Jul 15, 2024

Another iteration of tweaking the start-up message:

Was:

$ temporal server start-dev
Temporal server: localhost:7233
Web UI:          http://localhost:8233/
Metrics:         http://localhost:53881/metrics

Becomes:

$ temporal server start-dev
CLI 0.0.0-DEV (Server 1.24.1, UI 2.28.0)

Server:  localhost:7233
UI:      http://localhost:8233
Metrics: http://localhost:58550/metrics

@@ -394,6 +394,10 @@ func (c *TemporalCommand) initCommand(cctx *CommandContext) {
}
}

func VersionString() string {
return fmt.Sprintf("CLI %s (Server %s, UI %s)", Version, headers.ServerVersion, version.UIVersion)
Copy link
Member

Choose a reason for hiding this comment

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

Arguably we shouldn't show UI version if they haven't enabled the UI, but no big deal

@@ -144,11 +144,12 @@ func (t *TemporalServerStartDevCommand) run(cctx *CommandContext, args []string)
return err
}

cctx.Printer.Printlnf("%-16s %v:%v", "Temporal server:", toFriendlyIp(opts.FrontendIP), opts.FrontendPort)
cctx.Printer.Printlnf(VersionString() + "\n")
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
cctx.Printer.Printlnf(VersionString() + "\n")
cctx.Printer.Printlnf("%v\n", VersionString())

This is technically safer if there are accidentally any printf components in the version string

@dandavison dandavison merged commit 358f265 into temporalio:main Jul 15, 2024
7 checks passed
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.

2 participants