-
Notifications
You must be signed in to change notification settings - Fork 123
Propose command standards #2751
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
base: main
Are you sure you want to change the base?
Conversation
✅ Proposed changes verification passedThis pull request comes with up-to-date documentation and no illegal standard output usages. Find more detailed information in the |
|
|
||
| ## Command messaging | ||
|
|
||
| It's not allowed to use the `os.Stdout`/`os.Stderr` and `fmt.Print` functionalities. Instead of that we must use the [out](../../../internal/out/out.go) package that provide centralized control over writers. |
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.
Maybe in this section it would be useful to describe how specific out writers should be used? E.g. explain that out.Debug works only with the hidden --debug flag etc
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.
out.Verbose and out.Debug are almost identical from the code perspective but they serve different purposes
|
|
||
| The long one describes what exactly the command is doing. It can be multiline, describing all use-cases. It must start with a huge letter and always end with a period. | ||
|
|
||
| ## Flags |
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.
Do you think it would be worth mentioning in this section about flags that are available in every command? Or maybe adding another section describing Global flags
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.
Another thing: we haven't decide on the approach yet but it would be good to also standarize flags that are used across different commands like --format and --output
| explain [flags] # runnable command without argument | ||
| create <name> [flags] # runnable command that receive one required arg and optional flags | ||
| scale <name> <replicas> [flags] # runnable command that can receive two arguments and optional flags | ||
| delete <name\s> # runnable command that receive at least one argument and has no flags |
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 prefer something like <name>... or (<name>...). Se also kubectl get --help.
|
|
||
| The command name must be a verb or noun. In most cases, it is recommended to use nouns to define domain-related command groups (like `kyma function`, `kyma app`, or `kyma apirule`) and verbs to define runnable operations around the domain (like `kyma app push`, `kyma function create`, `kyma apirule expose`). This rule is only a suggestion, and it depends on the use case. For example, the `kyma diagnose` command works as a runnable command and command group at the same (`kyma diagnose logs`) time, and because of this, it's not a noun. On the other hand, the `kyma registry config` in another example, where after the noun is another noun (runnable noun), but in this case, the `config` word is shorter than `get-config`. | ||
|
|
||
| After the first word, there must be a description of possible arguments/commands. If the command is a non-runnable command group, then it should contain `<command>`, which means that this command accepts only one argument, and this argument is the command name. If command is runnable then is must describes possible inputs (if allowed) in the following format: `<arg_name>` for single, required argument, `<arg_name/s>` for at least one required argument, `[<arg_name>]` for one optional argument, `[<arg_name/s>]` for optional arguments list of the same type. If the command receives more than one argument type, then it is possible to describe many arguments separated by a space. For example: `scale <name> <replicas>` |
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.
Unification with the previous proposal:
| After the first word, there must be a description of possible arguments/commands. If the command is a non-runnable command group, then it should contain `<command>`, which means that this command accepts only one argument, and this argument is the command name. If command is runnable then is must describes possible inputs (if allowed) in the following format: `<arg_name>` for single, required argument, `<arg_name/s>` for at least one required argument, `[<arg_name>]` for one optional argument, `[<arg_name/s>]` for optional arguments list of the same type. If the command receives more than one argument type, then it is possible to describe many arguments separated by a space. For example: `scale <name> <replicas>` | |
| After the first word, there must be a description of possible arguments/commands. If the command is a non-runnable command group, then it should contain `<command>`, which means that this command accepts only one argument, and this argument is the command name. If command is runnable then is must describes possible inputs (if allowed) in the following format: `<arg_name>` for single, required argument, `(<arg_name>...)` for at least one required argument, `[<arg_name>]` for one optional argument, `[<arg_name>...]` for optional arguments list of the same type. If the command receives more than one argument type, then it is possible to describe many arguments separated by a space. For example: `scale <name> <replicas>` |
| The parents help: | ||
|
|
||
| ```bash | ||
| kyma module --help |
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.
An additional character ($) to distinguish the command from the result.
| kyma module --help | |
| $ kyma module --help |
| The commands help: | ||
|
|
||
| ```bash | ||
| kyma module catalog --help |
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.
| kyma module catalog --help | |
| $ kyma module catalog --help |
| The commands help looks: | ||
|
|
||
| ```bash | ||
| kyma module add --help |
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.
| kyma module add --help | |
| $ kyma module add --help |
| The help will displays: | ||
|
|
||
| ```bash | ||
| kyma module add --help |
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.
| kyma module add --help | |
| $ kyma module add --help |
|
|
||
| ### Name details | ||
|
|
||
| The command name must be a verb or noun. In most cases, it is recommended to use nouns to define domain-related command groups (like `kyma function`, `kyma app`, or `kyma apirule`) and verbs to define runnable operations around the domain (like `kyma app push`, `kyma function create`, `kyma apirule expose`). This rule is only a suggestion, and it depends on the use case. For example, the `kyma diagnose` command works as a runnable command and command group at the same (`kyma diagnose logs`) time, and because of this, it's not a noun. On the other hand, the `kyma registry config` in another example, where after the noun is another noun (runnable noun), but in this case, the `config` word is shorter than `get-config`. |
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.
Based on this, I assume that a verb or noun is a suggestion, not a must.
Actually, config can be used informally as configure, so it can be treated as a verb.
|
|
||
| After the first word, there must be a description of possible arguments/commands. If the command is a non-runnable command group, then it should contain `<command>`, which means that this command accepts only one argument, and this argument is the command name. If command is runnable then is must describes possible inputs (if allowed) in the following format: `<arg_name>` for single, required argument, `<arg_name/s>` for at least one required argument, `[<arg_name>]` for one optional argument, `[<arg_name/s>]` for optional arguments list of the same type. If the command receives more than one argument type, then it is possible to describe many arguments separated by a space. For example: `scale <name> <replicas>` | ||
|
|
||
| The last element must be optional flags represented by the `[flags]` element. In our case, it must be a part of every command because we add persistent flags for the parent `kyma` command, and these flags will be valid for every sub-command. |
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.
| The last element must be optional flags represented by the `[flags]` element. In our case, it must be a part of every command because we add persistent flags for the parent `kyma` command, and these flags will be valid for every sub-command. | |
| The last element must be optional flags represented by the `[flags]` element. In our case, it must be a part of every command because we add persistent flags for the parent `kyma` command, and these flags are valid for every sub-command. |
|
|
||
| ## Descriptions | ||
|
|
||
| There are two types of description under the `cobra.Command{}.Short` and the `cobra.Command{}.Long` fields. The first one represents a shorter description that will be shown when running parent commands help, and the second one when running the current command. |
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.
| There are two types of description under the `cobra.Command{}.Short` and the `cobra.Command{}.Long` fields. The first one represents a shorter description that will be shown when running parent commands help, and the second one when running the current command. | |
| There are two types of description under the `cobra.Command{}.Short` and the `cobra.Command{}.Long` fields. The first one represents a shorter description that is displayed when running parent commands help, and the second one is displayed when running the current command. |
|
|
||
| ### Description details | ||
|
|
||
| The short desc should help users choose the right sub-command in the context of the domain they are in. This description must start with a huge letter and end without a period. |
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.
| The short desc should help users choose the right sub-command in the context of the domain they are in. This description must start with a huge letter and end without a period. | |
| The short desc helps users choose the right sub-command in the context of the domain they are in. This description must start with a capital letter and end without a period. |
|
|
||
| The short desc should help users choose the right sub-command in the context of the domain they are in. This description must start with a huge letter and end without a period. | ||
|
|
||
| The long one describes what exactly the command is doing. It can be multiline, describing all use-cases. It must start with a huge letter and always end with a period. |
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.
| The long one describes what exactly the command is doing. It can be multiline, describing all use-cases. It must start with a huge letter and always end with a period. | |
| The longer one describes exactly what the command is doing. It can be multiline, describing all use-cases. It must start with a capital letter and always end with a period. |
|
|
||
| Every line of the examples must start with double spaces to display them correctly in the terminal. | ||
|
|
||
| All examples for one command must be separated with an empty line and have their own description starting from the `#` symbol. If the example description is longer than one line, then the first line should start with the `##` prefix, and then every next line should start with `#` and two spaces after. |
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.
| All examples for one command must be separated with an empty line and have their own description starting from the `#` symbol. If the example description is longer than one line, then the first line should start with the `##` prefix, and then every next line should start with `#` and two spaces after. | |
| All examples for a single command must be separated by an empty line and include their own description, starting with the `#` symbol. If the example description is longer than one line, then the first line should start with the `##` prefix, and then every next line should start with `#` and two spaces after. |
|
|
||
| All examples for one command must be separated with an empty line and have their own description starting from the `#` symbol. If the example description is longer than one line, then the first line should start with the `##` prefix, and then every next line should start with `#` and two spaces after. | ||
|
|
||
| Examples should reflect real use cases, so if possible, they should use real data as arguments and flags if possible. If not, then use fiction one representing real data, like `my-module`, `my-resource`, `my-something`. |
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.
| Examples should reflect real use cases, so if possible, they should use real data as arguments and flags if possible. If not, then use fiction one representing real data, like `my-module`, `my-resource`, `my-something`. | |
| Examples should reflect real use cases, so, if possible, they should use real data as arguments and flags. If not, then use fiction one representing real data, like `my-module`, `my-resource`, `my-something`. |
| } | ||
| ``` | ||
|
|
||
| ## Errors&Hints |
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.
| ## Errors&Hints | |
| ## Errors and Hints |
|
|
||
| ## Errors&Hints | ||
|
|
||
| Errors were proposed in the [ADR 001](001-error-output-format.md) proposal and implemented quite after that. This functionality allows users to understand what is going on on three levels of abstraction. The general message called `Error` should contain the last, user-understandable operation that fails. The second thing is `Error Details` that can contain an internal error message generated from the library or called server. The `Hints` section is something that should help users figure out how the problem can be fixed. Every hint should be in one of two formats: |
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.
| Errors were proposed in the [ADR 001](001-error-output-format.md) proposal and implemented quite after that. This functionality allows users to understand what is going on on three levels of abstraction. The general message called `Error` should contain the last, user-understandable operation that fails. The second thing is `Error Details` that can contain an internal error message generated from the library or called server. The `Hints` section is something that should help users figure out how the problem can be fixed. Every hint should be in one of two formats: | |
| Errors were proposed in the [ADR 001](001-error-output-format.md) proposal and implemented quite a while after that. With this functionality, users can understand what is going on at three levels of abstraction. The general message called `Error` should contain the last, user-understandable operation that fails. The second thing is `Error Details`, which contains an internal error message generated by the library or the server. The `Hints` section is designed to help users identify how to fix the problem. Every hint should be in one of two formats: |
| * `to <what>, <do>` - format used to describe possible optional configurations that may be used or misconfigured | ||
| * `make sure <what>` - format used to describe the required configuration user may have misconfigured | ||
|
|
||
| The cli is designed to always return `clierror.Error` instead of pure `error`. Both errors are not compatible with each other, and to avoid user confusion, we should not use the `cobra.Command{}.RunE` and instead of that use the `cobra.Command{}.Run` and check the error manually inside of it: |
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.
| The cli is designed to always return `clierror.Error` instead of pure `error`. Both errors are not compatible with each other, and to avoid user confusion, we should not use the `cobra.Command{}.RunE` and instead of that use the `cobra.Command{}.Run` and check the error manually inside of it: | |
| The CLI is designed to always return `clierror.Error` instead of pure `error`. Both errors are not compatible with each other, and to avoid user confusion, we should not use the `cobra.Command{}.RunE` and instead of that use the `cobra.Command{}.Run` and check the error manually inside of it: |
|
|
||
| ## Command messaging | ||
|
|
||
| It's not allowed to use the `os.Stdout`/`os.Stderr` and `fmt.Print` functionalities. Instead of that we must use the [out](../../../internal/out/out.go) package that provide centralized control over writers. |
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.
Maybe leave a small explanation why it's not allowed to use both os.Stdout/os.Stderr and fmt.Print functionalities.
Description
Changes proposed in this pull request: