Update command guidelines (#5769)

* Update command guidelines

format

fix md

* Address feedback

.github/PULL_REQUEST_TEMPLATE.md

@@ -2,13 +2,6 @@
 
 This checklist is used to make sure that common guidelines for a pull request are followed.
 
-### General Guidelines
-
 - [ ] The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see [Modifying change log](https://github.com/Azure/azure-cli/tree/master/doc/authoring_command_modules#modify-change-log)).
 
-### Command Guidelines
+- [ ] I adhere to the [Command Guidelines](../doc/command_guidelines.md).
-
-- [ ] Each command and parameter has a meaningful description.
-- [ ] Each new command has a test.
-
-(see [Authoring Command Modules](https://github.com/Azure/azure-cli/tree/master/doc/authoring_command_modules))

doc/authoring_command_modules/README.md

@@ -7,6 +7,8 @@ The document provides instructions and guidelines on how to author command modul
 
 **Command Authoring**:<br>https://github.com/Azure/azure-cli/blob/dev/doc/authoring_command_modules/authoring_commands.md
 
+**Command Guidelines**:<br>https://github.com/Azure/azure-cli/blob/dev/doc/command_guidelines.md
+
 **Help Authoring**:<br>https://github.com/Azure/azure-cli/blob/dev/doc/authoring_help.md
 
 **Test Authoring**:<br>https://github.com/Azure/azure-cli/blob/dev/doc/authoring_tests.md

doc/authoring_command_modules/authoring_commands.md

@@ -14,25 +14,23 @@ The document provides instructions and guidelines on how to author individual co
 
 **ADDITIONAL TOPICS**
 
-[6. Command Naming and Behavior Guidance](#command-naming-and-behavior-guidance)
+[6. Keyword Argument (kwarg) Reference](#keyword-argument-reference)
 
-[7. Keyword Argument (kwarg) Reference](#keyword-argument-reference)
+[7. Supporting the IDs Parameter](#supporting-the-ids-parameter)
 
-[8. Supporting the IDs Parameter](#supporting-the-ids-parameter)
+[8. Generic Update Commands](#generic-update-commands)
 
-[9. Generic Update Commands](#generic-update-commands)
+[9. Custom Table Formats](#custom-table-formats)
 
-[10. Custom Table Formats](#custom-table-formats)
+[10. Tab Completion (bash only)](#tab-completion)
 
-[11. Tab Completion (bash only)](#tab-completion)
+[11. Validators](#validators)
 
-[12. Validators](#validators)
+[12. Registering Flag Arguments](#registering-flags)
 
-[13. Registering Flag Arguments](#registering-flags)
+[13. Registering Enum Arguments](#registering-enums)
 
-[14. Registering Enum Arguments](#registering-enums)
+[14. Preventing particular extensions from being loading](#extension-suppression)
-
-[15. Preventing particular extensions from being loading](#extension-suppression)
 
 Authoring Commands
 =============================
@@ -287,75 +285,6 @@ Often reflected SDK methods have complex parameters that are difficult to expose
 Additional Topics
 =============================
 
-## Command Naming and Behavior Guidance
-
-**General Guidelines and Conventions**
-
-1. multi-word subgroups should be hyphenated (ex: `foo-resource` instead of `fooresource`)
-2. all command names should contain a verb (ex: `storage account get-connection-string` instead of `storage account connection-string`)
-3. avoid hyphenated command names when moving the commands into a subgroup would eliminate the need. (ex: instead of `show-database` and `list-database` use `database show` and `database get`.
-4. If a command subgroup would only have a single command, move it into the parent command group and hyphenate the name. This is common for commands which exist only to pull down cataloging information. (ex: instead of `database sku-definitions list` use `database list-sku-definitions`).
-5. Avoid command subgroups that have no commands. This often happens at the first level of a command branch. For example, KeyVault has secrets, certificates, etc that exist within a vault. The existing (preferred) CLI structure looks like:
-```
-Group
-    az keyvault: Safeguard and maintain control of keys, secrets, and certificates.
-
-Subgroups:
-    certificate  : Manage certificates.
-    key          : Manage keys.
-    secret       : Manage secrets.
-
-Commands:
-    create       : Create a key vault.
-    delete       : Delete a key vault.
-    delete-policy: Delete security policy settings for a Key Vault.
-    list         : List key vaults.
-    list-deleted : Gets information about the deleted vaults in a subscription.
-    purge        : Permanently deletes the specified vault.
-    recover      : Recover a key vault.
-    set-policy   : Update security policy settings for a Key Vault.
-    show         : Show details of a key vault.
-    update       : Update the properties of a key vault.
-```
-
-To create a vault, you simply use `az keyvault create ...`. An alternative would be to place the vault commands into a separate subgroup, like this:
-```
-Group
-    az keyvault: Safeguard and maintain control of keys, secrets, and certificates.
-
-Subgroups:
-    certificate  : Manage certificates.
-    key          : Manage keys.
-    secret       : Manage secrets.
-    vault        : Manage vaults.
-```
-
-Now, to create a vault, you have to use `az keyvault vault create ...` which is overly verbose adds unnecessary depth to the tree. The preferred style makes the command use more convenient and intuitive.
-
-**Standard Command Types**
-
-The following are standard names and behavioral descriptions for CRUD commands commonly found within the CLI.
-
-1. CREATE - standard command to create a new resource. Usually backed server-side by a PUT request. 'create' commands should be idempotent and should return the resource that was created.
-
-2. UPDATE - command to selectively update properties of a resource and preserve existing values. May be backed server-side by either a PUT or PATCH request, but the behavior of the command should *always* be PATCH-like. All `update` commands should be registerd using the `generic_update_command` helper to expose the three generic update properties. `update` commands MAY also allow for create-like behavior (PUTCH) in cases where a dedicated `create` command is deemed unnecessary. `update` commands should return the updated resource.
-
-3. SET - command to replace all properties of a resource without preserving existing values, typically backed server-side by a PUT request. This is used when PATCH-like behavior is deemed unnecessary and means that any properties not specifies are reset to their default values. `set` commands are more rare compared to `update` commands. `set` commands should return the updated resource.
-
-4. SHOW - command to show the properties of a resource, backed server-side by a GET request.
-
-5. LIST - command to list instances of a resource, backed server-side by a GET request. When there are multiple "list-type" commands within an SDK to list resources at different levels (for example, listing resources in a subscription vice in a resource group) the functionality should be exposed by have a single list command with arguments to control the behavior. For example, if `--resource-group` is provided, the command will call `list_by_resource_group`; otherwise, it will call `list_by_subscription`.
-
-6. DELETE - command to delete a resource, backed server-side by a DELETE request. Delete commands return nothing on success.
-
-**Non-standard Commands**
-
-For commands that don't conform to one of the above-listed standard command patterns, use the following guidance:
-
-1. Don't use single word verbs if they could cause confusion with the standard command types. For example, don't use `get` or `new` as these sound functionally the same as `show` and `create` respectively, leading to confusion as to what the expected behavior should be.
-2. Descriptive, hyphenated command names are often a better option than single verbs (ex: `vm assign-identity`, `vm perform-maintenance`).
-3. If in doubt, ask!
-
 ## Keyword Argument Reference
 
 **Overview of Keyword Arguments in Azure CLI 2.0**

doc/command_guidelines.md

@@ -0,0 +1,101 @@
+# Command Guidelines
+
+This document describes the command guidelines for 'az' and applies to both CLI command modules and extensions.
+
+Guidelines marked (*) only apply to command modules, not extensions.
+
+If in doubt, ask!
+
+## General Patterns
+
+- Be consistent with POSIX tools (support piping, work with grep, awk, jq, etc.)
+- Support tab completion for parameter names and values (e.g. resource names)
+- All commands, command group and arguments must have descriptions
+- You must provide command examples for non-trivial commands
+- All commands must support all output types (json, tsv, table)
+- Provide custom table outputs for commands that don't provide table output automatically
+- Commands must return an object, dictionary or `None` (do not string, Boolean, etc. types)
+- Command output must go to `stdout`, everything else to `stderr` (log/status/errors).
+- Log to `logger.error()` or `logger.warning()` for user messages; do not use the `print()` function
+- Use the appropriate logging level for printing strings. e.g. `logging.info(“Upload of myfile.txt successful”)` NOT `return “Upload successful”`.
+
+## Command Naming and Behavior
+
+- Commands must follow a "[noun] [noun] [verb]" pattern
+- Multi-word subgroups should be hyphenated
+e.g. `foo-resource` instead of `fooresource`
+- All command names should contain a verb
+e.g. `account get-connection-string` instead of `account connection-string`
+- Avoid hyphenated command names when moving the commands into a subgroup would eliminate the need.
+e.g. `database show` and `database get` instead of `show-database` and `get-database`
+- If a command subgroup would only have a single command, move it into the parent command group and hyphenate the name. This is common for commands which exist only to pull down cataloging information.
+e.g. `database list-sku-definitions` instead of `database sku-definitions list`
+- Avoid command subgroups that have no commands. This often happens at the first level of a command branch.
+e.g. `keyvault create` instead of `keyvault vault create` (where `keyvault` only has subgroups and adds unnecessary depth to the tree).<details>
+  <summary>Click for a full example</summary>
+  <p>
+  KeyVault has secrets, certificates, etc that exist within a vault. The existing (preferred) CLI structure looks like:	
+
+    ```	
+    Group	
+        az keyvault: Safeguard and maintain control of keys, secrets, and certificates.	
+        
+    Subgroups:	
+        certificate  : Manage certificates.	
+        key          : Manage keys.	
+        secret       : Manage secrets.	
+        
+    Commands:	
+        create       : Create a key vault.	
+        delete       : Delete a key vault.	
+        delete-policy: Delete security policy settings for a Key Vault.	
+        list         : List key vaults.	
+        list-deleted : Gets information about the deleted vaults in a subscription.	
+        purge        : Permanently deletes the specified vault.	
+        recover      : Recover a key vault.	
+        set-policy   : Update security policy settings for a Key Vault.	
+        show         : Show details of a key vault.	
+        update       : Update the properties of a key vault.	
+    ```	
+        
+    To create a vault, you simply use `az keyvault create ...`. An alternative would be to place the vault commands into a separate subgroup, like this:	
+    ```	
+    Group	
+        az keyvault: Safeguard and maintain control of keys, secrets, and certificates.	
+        
+    Subgroups:	
+        certificate  : Manage certificates.	
+        key          : Manage keys.	
+        secret       : Manage secrets.	
+        vault        : Manage vaults.	
+    ```	
+        
+    Now, to create a vault, you have to use `az keyvault vault create ...` which is overly verbose adds unnecessary depth to the tree. The preferred style makes the command use more convenient and intuitive.
+</p>
+</details>
+
+## Standard Command Types
+
+The following are standard names and behavioral descriptions for CRUD commands commonly found within the CLI. These standard command types MUST be followed for consistency with the rest of the CLI.
+
+- `CREATE` - standard command to create a new resource. Usually backed server-side by a PUT request. 'create' commands should be idempotent and should return the resource that was created.
+- `UPDATE` - command to selectively update properties of a resource and preserve existing values. May be backed server-side by either a PUT or PATCH request, but the behavior of the command should always be PATCH-like. All `update` commands should be registered using the `generic_update_command` helper to expose the three generic update properties. `update` commands MAY also allow for create-like behavior (PUTCH) in cases where a dedicated `create` command is deemed unnecessary. `update` commands should return the updated resource.
+- `SET` - command to replace all properties of a resource without preserving existing values, typically backed server-side by a PUT request. This is used when PATCH-like behavior is deemed unnecessary and means that any properties not specifies are reset to their default values. `set` commands are more rare compared to `update` commands. `set` commands should return the updated resource.
+- `SHOW` - command to show the properties of a resource, backed server-side by a GET request.
+- `LIST` - command to list instances of a resource, backed server-side by a GET request. When there are multiple "list-type" commands within an SDK to list resources at different levels (for example, listing resources in a subscription vice in a resource group) the functionality should be exposed by have a single list command with arguments to control the behavior. For example, if `--resource-group` is provided, the command will call `list_by_resource_group`; otherwise, it will call `list_by_subscription`.
+- `DELETE` - command to delete a resource, backed server-side by a DELETE request. Delete commands return nothing on success.
+
+## Non-standard Commands
+
+For commands that don't conform to one of the above-listed standard command patterns, use the following guidance.
+
+- (*) Don't use single word verbs if they could cause confusion with the standard command types. For example, don't use `get` or `new` as these sound functionally the same as `show` and `create` respectively, leading to confusion as to what the expected behavior should be.
+- (*) Descriptive, hyphenated command names are often a better option than single verbs.
+
+## Coding Practices
+
+- All code must support Python 2 & 3.
+The CLI supports 2.7, 3.4, 3.5 and 3.6
+- PRs to Azure/azure-cli and Azure/azure-cli-extensions must pass CI
+- (*) Code must pass style checks with pylint and pep8
+- (*) All commands should have tests

doc/extensions/README.md

@@ -28,6 +28,8 @@ Doc Sections
 
 - [Authoring](authoring.md) - How to author and develop an extension
 
+- [Command Guidelines](../command_guidelines.md) - Command Guidelines
+
 - [Publishing](publishing.md) - How to publish an extension
 
 - [Extension Metadata](metadata.md) - How to add additional extension metadata