Skip to content
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

Add descriptions for cloud commands and params #2018

Merged
merged 1 commit into from Feb 10, 2017

Conversation

derekbekoe
Copy link
Member

No description provided.

Copy link
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would prefer defaulting the cloud_name to the active cloud name if feasible rather than explanatory text.

@@ -21,14 +21,40 @@ def get_custom_cloud_name_completion_list(prefix, action, parsed_args, **kwargs)
completer=get_cloud_name_completion_list)

register_cli_argument('cloud show', 'cloud_name',
help='Name of the registered cloud. If not specified, we use the current active cloud.')
help='Name of the registered cloud. If not specified, we use the active cloud.')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not simply have the default value get the active cloud name (like VM admin username)? Then we don't need this "If not specified..." text and it will be clear what the active cloud is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion!
Implemented.

…ashed commit)

Squashed commits:
[82d3fe9] Add descriptions for cloud commands and params
@@ -6,7 +6,7 @@

from azure.cli.core.commands import register_cli_argument

from azure.cli.core.cloud import get_clouds, get_custom_clouds
from azure.cli.core.cloud import get_clouds, get_custom_clouds, get_active_cloud_name


# pylint: disable=line-too-long
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if nothing is violate this rule, can we remove it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_params.py (and commands.py) are the main cases where we deliberately keep this in. We went back in forth on the single-line/multi-line thing and ultimately went the route of single line, which all but guarantees this Pylint check should be disabled.

@derekbekoe derekbekoe merged commit c86321c into Azure:master Feb 10, 2017
@derekbekoe derekbekoe deleted the cloud-descs branch February 10, 2017 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants