Conversation
|
@mafredri — this is probably 1k lines excluding generated code. I've manually tested that it works, but I'll write YAML docs and restore the support links docs in a separate PR with @bpmct to review. I also agree with making OptionSet a Edit: untagged you since you're on PTO. |
| comment := wordwrap.WrapString( | ||
| fmt.Sprintf("%s (default: %s, type: %s)", opt.Description, defValue, opt.Value.Type()), | ||
| 80, | ||
| ) | ||
| nameNode := yaml.Node{ | ||
| Kind: yaml.ScalarNode, | ||
| Value: opt.YAML, | ||
| HeadComment: wordwrap.WrapString(opt.Description, 80), | ||
| HeadComment: wordwrap.WrapString(comment, 80), |
There was a problem hiding this comment.
comment seems to be wordwrap'd twice
| defValue = "<unset>" | ||
| } | ||
| comment := wordwrap.WrapString( | ||
| fmt.Sprintf("%s (default: %s, type: %s)", opt.Description, defValue, opt.Value.Type()), |
There was a problem hiding this comment.
I think this would be nicer to read as %s\n(default: %s, type: %s) so default is always on the last line. Looking at the golden file it was hard to parse when the default/type randomly wrapped at 80 chars.
There was a problem hiding this comment.
Big fan of including the description as a comment, and the word wrapping makes it easy to read.
| # An HTTP URL that is accessible by other replicas to relay DERP traffic. Required | ||
| # for high availability. | ||
| # (default: <unset>, type: url) | ||
| relayURL: |
There was a problem hiding this comment.
It seems like url types are the only fields that are printed out empty when the default is unset, the others seem to use the zero value.
There was a problem hiding this comment.
Maybe the type would be better as string(url) so that it's clear you're inputting a string here? Unsure though.
There was a problem hiding this comment.
I think for the most part it's self-explanatory.
This re-releases support links, amongst other benefits to operability.
Follow-up:
[]*Optionto avoid all the complex indirection seen in this PR