command line flags: dashes and underscores synonyms#7406
command line flags: dashes and underscores synonyms#7406shlomi-noach merged 7 commits intovitessio:masterfrom
Conversation
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
| flag.DurationVar(&heartbeatInterval, "heartbeat_interval", 1*time.Second, "How frequently to read and write replication heartbeat.") | ||
| flag.BoolVar(¤tConfig.EnableLagThrottler, "enable-lag-throttler", defaultConfig.EnableLagThrottler, "If true, vttablet will run a throttler service, and will implicitly enable heartbeats") | ||
| flag.BoolVar(¤tConfig.EnableLagThrottler, "enable_lag_throttler", defaultConfig.EnableLagThrottler, "If true, vttablet will run a throttler service, and will implicitly enable heartbeats") | ||
| flag.BoolVar(¤tConfig.EnableLagThrottler, "enable-lag-throttler", currentConfig.EnableLagThrottler, "synonym to -enable_lag_throttler") |
There was a problem hiding this comment.
notice both the target and the default are currentConfig.EnableLagThrottler
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
| flag.BoolVar(&enableHeartbeat, "heartbeat_enable", false, "If true, vttablet records (if master) or checks (if replica) the current time of a replication heartbeat in the table _vt.heartbeat. The result is used to inform the serving state of the vttablet via healthchecks.") | ||
| flag.DurationVar(&heartbeatInterval, "heartbeat_interval", 1*time.Second, "How frequently to read and write replication heartbeat.") | ||
| flag.BoolVar(¤tConfig.EnableLagThrottler, "enable-lag-throttler", defaultConfig.EnableLagThrottler, "If true, vttablet will run a throttler service, and will implicitly enable heartbeats") | ||
| flag.BoolVar(¤tConfig.EnableLagThrottler, "enable_lag_throttler", currentConfig.EnableLagThrottler, "synonym to -enable-lag-throttler") |
There was a problem hiding this comment.
If both are specified, which one wins?
In order to standardize on underscore, should we document the _ version of the flag and call the other one a synonym?
There was a problem hiding this comment.
in the above case, underscores override dashes. Or, actually, I think that just depends on the order specified on the command line (with any single-valued flag, you can specify it as many times you want on the CLI and the last one wins it all).
In order to standardize on underscore, should we document the _ version of the flag and call the other one a synonym?
Agreed.
There was a problem hiding this comment.
BTW confirmed, last flag specified on the command line wins, regardless of which order the flags are created in code.
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
|
reversed order: underscores is the official flag, dashes is the synonym. |
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
|
Refactor: extracted the dual-case logic to |
deepthi
left a comment
deepthi
left a comment
There was a problem hiding this comment.
This is really cool. You may merge once the feedback is addressed.
go/flagutil/flagutil.go
Outdated
| return strings.Join(parts, ",") | ||
| } | ||
|
|
||
| // DualCaseStringListVar cretes a flag which supports both dashes and underscores |
There was a problem hiding this comment.
typo: cretes -> creates (applies to all the functions)
go/flagutil/flagutil.go
Outdated
| } | ||
|
|
||
| // DualCaseStringListVar cretes a flag which supports both dashes and underscores | ||
| func DualCaseStringListVar(p *[]string, name string, value []string, usage string) { |
There was a problem hiding this comment.
Sorry to be nitpicky, but DualCase to me means upper/lower case. Here we are talking about two different formats. I suggest renaming these functions to DualFormat...
There was a problem hiding this comment.
Renaming. I used the term "Case" because it's common to refer to this as snake_case and kebab-case.
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
3 participants
Description
As pointed by @deepthi here we use both dash-based and underscore-based command line flags. This PR show how we can make synonyms so we can standardize on one (underscored) over the other (dashes)
Related Issue(s)
See #7405
Checklist
Impacted Areas in Vitess
Components that this PR will affect: