OCM-1888: Add docs for ocm delete account command #592
Conversation
cgiradkar
changed the title
cgiradkar
force-pushed
the
OCM-1888-ocm-delete-account
branch
3 times, most recently
from
31e252d to
b841440
Compare
cgiradkar
changed the title
cgiradkar
force-pushed
the
OCM-1888-ocm-delete-account
branch
from
b841440 to
6b6c601
Compare
cmd/ocm/delete/cmd.go
Outdated
| @@ -40,14 +40,49 @@ var args struct { | |||
| } | |||
|
|
|||
| var Cmd = &cobra.Command{ | |||
| Use: "delete [flags] PATH", | |||
| Use: "delete [flags] PATH|RESOURCE_ALIAS(with resource_id)", | |||
There was a problem hiding this comment.
I think we should be a bit more aligned with common documentation practices around command lines.
I propose two possible approaches:
One approach is listing the alternative uses:
delete [flags] PATH
delete [flags] RESOURCE_ALIAS RESOURCE_ID
The other is documenting it in a single line:
delete [flags] {PATH|RESOURCE_ALIAS RESOURCE_ID}
There was a problem hiding this comment.
I would suggest following the convention used by oc openshift CLI
There was a problem hiding this comment.
oc / kubectl seems to use the single line approach (at least quickly checking some examples in there like oc label), with a different character for grouping:
I think it should be in the following way then:
delete [flags] (PATH | RESOURCE_ALIAS RESOURCE_ID)
6b6c601 to
25c273b
Compare
cgiradkar
changed the title
cgiradkar
force-pushed
the
OCM-1888-ocm-delete-account
branch
from
25c273b to
5c6b11a
Compare
cmd/ocm/delete/cmd.go
Outdated
| Short: "Send a DELETE request", | ||
| Long: "Send a DELETE request to the given path.", | ||
| RunE: run, | ||
| ValidArgs: urls.Resources(), | ||
| } | ||
|
|
||
| var usageTemplate = `Usage:{{if .Runnable}} |
There was a problem hiding this comment.
If we will adapt this template for other command we should move it to some utils so we don't need to move it when doing refactor of the other commands.
There was a problem hiding this comment.
this template was picked up from cobra's internal help/usage template function: https://github.com/spf13/cobra/blob/bcfcff729ecdeb4072ef6f2a9c0b5e4ca1853206/command.go#L546
There was a problem hiding this comment.
We discussed and decided to do it just for delete command and if it works properly without any breakages, then we can expand the scope. Also, it should not be a big task to make the template centralized.
cgiradkar
force-pushed
the
OCM-1888-ocm-delete-account
branch
from
5c6b11a to
2a44273
Compare
|
Studied the template format from: https://pkg.go.dev/text/template to make necessary changes. Final o/p: |
|
Added a 2nd commit with format in string literal. Both commits to be compared side by side. @miguelsorianod , can you please compare and lets decide on what's the best format to merge? |
|
If the different parts of the template are not reused anywhere else I am fine with having it as a literal |
Prevously when using the help in OCM-CLI it did not list all available sub commands. This was due to how we currently alias urls in the CLI. Now added documentation about using the urls(path) or provide a resource_id for the resource to be deleted. Another effort has been started to understand and standardize the help format, which in turn will help in the task OCM-5179. The template format has been used from https://pkg.go.dev/text/template. Signed-off-by: Chetan Giradkar <[email protected]>
cgiradkar
force-pushed
the
OCM-1888-ocm-delete-account
branch
from
21f0b47 to
3423d52
Compare
|
As discussed, I have used string literal format and added back the 2nd usage ( |
|
It looks good to me. I am assuming the output looks like the one shown in #592 (comment). If that's not the case please upload a new capture to show the result. |
Prevously when using the help in OCM-CLI it did not list all available sub commands.
This was due to how we currently alias urls in the CLI. Now added documentation about using the
urls(path) or provide a resource_id for the resource to be deleted.
Signed-off-by: Chetan Giradkar [email protected]