CLI Refresh: Output improvements on start-dev #441
Conversation
| s.Command.Flags().StringVar(&s.UiIp, "ui-ip", "", "IP address to bind the Web UI to. Default is same as --ip.") | ||
| s.Command.Flags().StringVar(&s.UiAssetPath, "ui-asset-path", "", "UI custom assets path.") | ||
| s.Command.Flags().StringVar(&s.UiCodecEndpoint, "ui-codec-endpoint", "", "UI remote codec HTTP endpoint.") | ||
| s.Command.Flags().StringArrayVar(&s.SqlitePragma, "sqlite-pragma", nil, "Specify SQLite pragma statements in pragma=value format.") | ||
| s.Command.Flags().StringArrayVar(&s.DynamicConfigValue, "dynamic-config-value", nil, "Dynamic config value, as KEY=JSON_VALUE (string values need quotes).") | ||
| s.Command.Flags().BoolVar(&s.LogConfig, "log-config", false, "Log the server config being used in stderr.") | ||
| s.LogLevelServer = NewStringEnum([]string{"debug", "info", "warn", "error", "off"}, "warn") | ||
| s.Command.Flags().Var(&s.LogLevelServer, "log-level-server", "Log level for the server only.") |
There was a problem hiding this comment.
Not related to this PR but I think it would be nice to show the user the available alternative enum values
--log-level-server string Log level for the server only. (default "warn")
There was a problem hiding this comment.
Does --log-level off have precedent elsewhere in Temporal/Go? If so then fine; if not, then I'll also throw in --log-level never as a candidate, since it's sort of consistent gramatically: debug, info, warn, error, never.
There was a problem hiding this comment.
Not related to this PR but I think it would be nice to show the user the available alternative enum values
Agreed. We currently have a general concept of trailing options attribute sentences being removed from the English part, and Cobra doesn't support any kind of enum listing. So we can, at code writing time, append the "Accepted values: <values...>" for all enums.
Does --log-level off have precedent elsewhere in Temporal/Go?
No, only here I think, so I'd support changing this to never for sure.
I can open a PR for both of these shortly...
|
This is a huge improvement. |
What was changed
localhostand translate internally to 127.0.0.1 for use but keep as localhost for text outputNow when you run
temporal server start-dev, this is the output:(many suggestions came from @lorensr)