Add vtctldclient docs by ajm188 · Pull Request #1054 · vitessio/website

ajm188 · 2022-06-13T14:03:32Z

This adds docs for vtctldclient autogenerated via cobra/doc.GenMarkdownTreeCustom. I'm working on generalizing the backing code, but I think me just committing this is fine for 14.0 (but when we start moving more commands over in the next release, we should 100% have this tooling integrated).

Anyway, here's the steps (from vitessio/vitess):

➜  vitess git:(andrew/vtctldclient/autodoc) ✗ go build -o docgen ./go/cmd/vtctldclient/docgen
➜  vitess git:(andrew/vtctldclient/autodoc) ✗ ./docgen -d ./go/cmd/vtctldclient/doc
➜  vitess git:(andrew/vtctldclient/autodoc) ✗ mv ./go/cmd/vtctldclient/doc/vtctldclient.md ./go/cmd/vtctldclient/doc/_index.md
➜  vitess git:(andrew/vtctldclient/autodoc) ✗ cp ./go/cmd/vtctldclient/doc/*.md ~/dev/vtwebsite/content/en/docs/14.0/reference/programs/vtctldclient

(when I publish the tooling, that'll all be in a make target)

Signed-off-by: Andrew Mason <andrew@planetscale.com>

netlify · 2022-06-13T14:06:16Z

✅ Deploy Preview for vitess ready!

Name	Link
🔨 Latest commit	`2ca675c`
🔍 Latest deploy log	https://app.netlify.com/sites/vitess/deploys/62a8d00a2d5c5f00097b1e22
😎 Deploy Preview	https://deploy-preview-1054--vitess.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

GuptaManan100

GitHub is making this impossible to review! But I ❤️ it. Do we want one file for each command though or 1 long file?

ajm188 · 2022-06-13T15:36:23Z

GitHub is making this impossible to review!

I'd recommend just looking at the deploy preview!! Every file is "the same" (except for vtctldclient which contains the summary).

Do we want one file for each command though or 1 long file?

This is the standard way that cobra auto doc works, so I'd rather stick with that than try to shoe-horn it into some other setup unless we have a specific reason to do so.

deepthi · 2022-06-13T22:45:32Z

Clicking links on the preview is returning 404.
E.g. https://deploy-preview-1054--vitess.netlify.app/docs/14.0/reference/programs/vtctldclient/vtctldclient_AddCellInfo.md

deepthi · 2022-06-13T22:48:12Z

content/en/docs/14.0/reference/programs/vtctldclient/vtctldclient_GetBackups.md

+---
+## vtctldclient GetBackups
+
+


Some commands like this one show an empty description on the preview. I think we need to add the right text to the source and regenerate.

yeah, i can go through the actual commands in vitessio/vitess tomorrow and add all those

will that be a follow up PR? If yes, this LGTM

opened: vitessio/vitess#10502. after addressing any feedback there, i'll regenerate and we can merge this one!

ajm188 · 2022-06-13T22:54:23Z

Clicking links on the preview is returning 404.
E.g. https://deploy-preview-1054--vitess.netlify.app/docs/14.0/reference/programs/vtctldclient/vtctldclient_AddCellInfo.md

ah shoot, i see the problem, will fix it up!

Signed-off-by: Andrew Mason <andrew@planetscale.com>

mattlord

This is friggin awesome! 😃

Is --server really optional with vtctldclient? I see some commands imply that it is, e.g.

vtctldclient UpdateCellInfo [--root <root>] [--server-address <addr>] <cell>

And some commands don't mention it at all, e.g.:

vtctldclient DeleteKeyspace [--recursive|-r] [--force|-f] <keyspace> [flags]

Although for the latter I now see that it mentions those separately as being inherited. So maybe that part is fine (although I'd expect those to be in the usage output too).

ajm188 · 2022-06-14T15:41:18Z

This is friggin awesome! 😃

Is --server really optional with vtctldclient? I see some commands imply that it is, e.g.
vtctldclient UpdateCellInfo [--root <root>] [--server-address <addr>] <cell>
And some commands don't mention it at all, e.g.:
vtctldclient DeleteKeyspace [--recursive|-r] [--force|-f] <keyspace> [flags]
Although for the latter I now see that it mentions those separately as being inherited. So maybe that part is fine (although I'd expect those to be in the usage output too).

You're mistaking the --server-address argument which is for only {Add,Update}CellInfo with the global --server flag that's defined at the command root.

--server lives here and is required

ajm188 · 2022-06-14T15:43:20Z

It also does appear in each command's docs, e.g. https://deploy-preview-1054--vitess.netlify.app/docs/14.0/reference/programs/vtctldclient/vtctldclient_addcellinfo/#options-inherited-from-parent-commands

Signed-off-by: Andrew Mason <andrew@planetscale.com>

ajm188 · 2022-06-14T18:15:42Z

@deepthi okay i regenerated (and added the 15.0 versions), this should be good to go!

mattlord · 2022-06-14T18:25:49Z

This is friggin awesome! 😃
Is --server really optional with vtctldclient? I see some commands imply that it is, e.g.
vtctldclient UpdateCellInfo [--root <root>] [--server-address <addr>] <cell>
And some commands don't mention it at all, e.g.:
vtctldclient DeleteKeyspace [--recursive|-r] [--force|-f] <keyspace> [flags]
Although for the latter I now see that it mentions those separately as being inherited. So maybe that part is fine (although I'd expect those to be in the usage output too).
You're mistaking the --server-address argument which is for only {Add,Update}CellInfo with the global --server flag that's defined at the command root.

Ah, right. Sorry!

--server lives here and is required

It doesn't seem to be required on the command pages though, the command usage doesn't show it without square brackets to indicate it's non-optional and in the inherited section it says:

--server string             server to use for connection

I'd prefer to show those inherited things in the full usage output for each command, but can we at least add e.g. --server string server to use for connection (required) as I don't see an indication that required is the default unless optional is specified either.

It's a nit obviously, and doesn't need to hold this up (we have no docs ATM so this is obviously much better). I'll approve so that you're unblocked. Thanks!

mattlord · 2022-06-14T18:29:47Z

One general note, not specifically related to this PR, is that we don't seem to have a uniform way of showing usage related info when you must specify 1 of N options. For example, in some cases we use ~ this to signify that one of the two is required:

vtctldclient UpdateCellInfo {--root <root> | --server-address <addr>} <cell>

We can take the lead from pflags on that and update the docs everywhere to match over time.

ajm188 · 2022-06-14T18:40:06Z

lemme fix that real quick!

as for the standardization, totally agree. it's grown organically over time, and before v15 release we should sit down (metaphorically 😂 ) and articulate what conventions we want to stick to (and then fix all my sins)

deepthi

LGTM

ajm188 · 2022-06-14T18:49:41Z

piled on the --server change in #1055 (specifically c9eff41)

Andrew Mason added 2 commits June 13, 2022 09:59

Update layouts to exclude vtctldclient from Pages-in-Section

f48c145

Signed-off-by: Andrew Mason <andrew@planetscale.com>

Add auto-generated vtctldclient docs

83bdd9a

Signed-off-by: Andrew Mason <andrew@planetscale.com>

ajm188 requested review from deepthi and mattlord June 13, 2022 14:03

GuptaManan100 reviewed Jun 13, 2022

View reviewed changes

ajm188 mentioned this pull request Jun 13, 2022

Add docs for grpc-vtctld service #1055

Merged

deepthi reviewed Jun 13, 2022

View reviewed changes

Fix linking

837b201

Signed-off-by: Andrew Mason <andrew@planetscale.com>

ajm188 mentioned this pull request Jun 14, 2022

[vtctldclient] Update CLI docs for usages, flags, and aliases vitessio/vitess#10502

Merged

3 tasks

mattlord reviewed Jun 14, 2022

View reviewed changes

Andrew Mason added 2 commits June 14, 2022 14:13

Update docs after regenerating

f7d657d

Signed-off-by: Andrew Mason <andrew@planetscale.com>

Also copy to 15.0

2ca675c

Signed-off-by: Andrew Mason <andrew@planetscale.com>

mattlord approved these changes Jun 14, 2022

View reviewed changes

deepthi approved these changes Jun 14, 2022

View reviewed changes

deepthi merged commit eeb321e into prod Jun 14, 2022

deepthi deleted the andrew/vtctldclient-docs branch June 14, 2022 18:40

ajm188 mentioned this pull request Jun 15, 2022

Fix section page filtering partials #1058

Merged

GuptaManan100 mentioned this pull request Jul 14, 2022

Add Vtctld commands page to the website #1045

Closed

Conversation

ajm188 commented Jun 13, 2022

Uh oh!

netlify bot commented Jun 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for vitess ready!

Uh oh!

GuptaManan100 left a comment

Choose a reason for hiding this comment

Uh oh!

ajm188 commented Jun 13, 2022

Uh oh!

deepthi commented Jun 13, 2022

Uh oh!

deepthi Jun 13, 2022

Choose a reason for hiding this comment

Uh oh!

ajm188 Jun 13, 2022

Choose a reason for hiding this comment

Uh oh!

deepthi Jun 14, 2022

Choose a reason for hiding this comment

Uh oh!

ajm188 Jun 14, 2022

Choose a reason for hiding this comment

Uh oh!

ajm188 commented Jun 13, 2022

Uh oh!

mattlord left a comment

Choose a reason for hiding this comment

Uh oh!

ajm188 commented Jun 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ajm188 commented Jun 14, 2022

Uh oh!

ajm188 commented Jun 14, 2022

Uh oh!

mattlord commented Jun 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattlord commented Jun 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ajm188 commented Jun 14, 2022

Uh oh!

deepthi left a comment

Choose a reason for hiding this comment

Uh oh!

ajm188 commented Jun 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

netlify bot commented Jun 13, 2022 •

edited

Loading

ajm188 commented Jun 14, 2022 •

edited

Loading

mattlord commented Jun 14, 2022 •

edited

Loading

mattlord commented Jun 14, 2022 •

edited

Loading