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

Configuration page rework: remove duplication, make easier to maintain, add missing flag #491

Merged
merged 7 commits into from
Oct 26, 2021

Conversation

chalin
Copy link
Contributor

@chalin chalin commented Sep 20, 2021

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:

  • Reworks and simplifies the 3.5 Configuration page to, in particular, remove duplication and make it easier to maintain.
  • For now, I've opted to show the configuration flags as they're output by the etcd -h command, which make maintenance much easier.
  • Addresses Add doc for --initial-election-tick-advance flag #433 for the v3.5 page
  • Also removes obsolete flags.

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/

@chalin
Copy link
Contributor Author

chalin commented Sep 20, 2021

Update: in order to preserve the ability to link into flag sections, I've split the etcd -h output into markdown sections (manually, ideally we'll like to get a shortcode and/or script to do this -- but that'll be for later).

@chalin chalin force-pushed the chalin-v3.5-config-2021-09-20 branch from 6b5da88 to bd56555 Compare September 20, 2021 15:29
@chalin chalin force-pushed the chalin-v3.5-config-2021-09-20 branch from 72580b8 to 1b7b818 Compare September 20, 2021 19:24
Copy link
Contributor

@nate-double-u nate-double-u left a 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

@chalin
Copy link
Contributor Author

chalin commented Sep 20, 2021

Thanks for the review, and the suggested change (which I've accepted).

@chalin
Copy link
Contributor Author

chalin commented Sep 20, 2021

@spzala - WDYT?

Copy link
Member

@spzala spzala left a 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
Copy link
Member

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!

Copy link
Contributor Author

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.

Copy link
Contributor Author

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?

@chalin
Copy link
Contributor Author

chalin commented Oct 26, 2021

I'm going to merge based on @spzala's LGTM in #491 (review), and then open a followup issue for #491 (comment).

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.

3 participants