-
Notifications
You must be signed in to change notification settings - Fork 305
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
Configuration page rework: remove duplication, make easier to maintain, add missing flag #491
Conversation
Update: in order to preserve the ability to link into flag sections, I've split the |
6b5da88
to
bd56555
Compare
72580b8
to
1b7b818
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this organization @chalin, thanks for this.
I've put one nit inline regarding spacing. Otherwise, /lgtm
Co-authored-by: Nate W. <[email protected]>
Thanks for the review, and the suggested change (which I've accepted). |
@spzala - WDYT? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have one inline comment but other than that lgtm
Thanks @chalin @nate-double-u
--experimental-distributed-tracing-instance-id '' | ||
Distributed tracing instance ID, must be unique per each etcd instance. | ||
``` | ||
### v2 Proxy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we keep it just "Proxy" instead of v2 Proxy.. (similar to how it's originally mentioned ## Proxy flags) Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I mentioned in the opening comment:
- I've opted to show the configuration flags as they're output by the
etcd -h
command, which make maintenance much easier.
The "v2 Proxy" comes from the command's help text. I'd rather match the help text (given that we're aiming to have this page generated automatically from the help text), but if you feel strongly about it I can make the change. Please let me know. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, @spzala, I'll let you answer here. Thanks.
Maybe we should open an issue an issue over the etcd repo to have the flag group title changes?
I'm going to merge based on @spzala's LGTM in #491 (review), and then open a followup issue for #491 (comment). |
The Configuration pages for v3.5 and v.34 combined get ~2% traffic relative to the overall site: my guess is that most users simply run
etcd -h
.This PR:
etcd -h
command, which make maintenance much easier.Later we could consider writing a shortcode to format the command-line flag information more nicely.
Preview: https://deploy-preview-491--etcd.netlify.app/docs/v3.5/op-guide/configuration/