arm review: definition and assignment command categories should be sxs #1041
Conversation
yugangw-msft
force-pushed
the
armreview1
branch
2 times, most recently
from
83f734a to
f88bed4
Compare
| raise CLIError(err.format(resource_group_name)) | ||
| scope = resource_id | ||
| scope = scope |
There was a problem hiding this comment.
scope = scope?
This isn't needed right?
| cli_command('resource policy list', PolicyDefinitionsOperations.list, factory) | ||
| cli_command('resource policy show', PolicyDefinitionsOperations.get, factory) | ||
| cli_command('resource policy update', update_policy_definition) | ||
| cli_command('resource policy definition create', create_policy_definition) |
There was a problem hiding this comment.
Is this going to create a group that only has one subgroup and no commands? If so, we should use the hyphenated version resource policy-definition ... instead. If resource policy has commands of its own then this is fine.
There was a problem hiding this comment.
The PR just moved all policy definition commands from az resource policy to az resource policy definitions. So there are no commands under az resource policy now, rather just 2 groups, defintion and assignment. ARM team prefers this layout, and i agree.
| cli_command('role delete', delete_role_definition) | ||
| cli_command('role create', create_role_definition) | ||
| cli_generic_update_command('role update', | ||
| cli_command('role definition list', list_role_definitions) |
There was a problem hiding this comment.
Same comment here. If there are no commands under role, only the definition subgroup, then we should use a hyphenated version role-definition instead.
There was a problem hiding this comment.
Same with resource policy, role has 2 sub-groups definition and assignment
yugangw-msft
force-pushed
the
armreview1
branch
from
4b9e02e to
91784c0
Compare
fix partial #1033. I moved definition related commands to under
role definitionorresource policy definition. Also fix some argument naming and completer related issuesfix #1020