Skip to content

Conversation

@derekbekoe
Copy link
Member

@digimaun Please review. You can use the prompt.ws service below to do some ad-hoc testing of the commands (It takes 2/3 minutes to load so refresh the page afterwards).

@azuresdkci
Copy link
Contributor

View a preview at https://prompt.ws/r/Azure/azure-cli/5084
This is an experimental preview for @microsoft.com users.
(It may take a minute or two for your instance to be ready)
Email feedback to 'azfeedback' with subject 'Prompt Feedback'.

Copy link
Member Author

Choose a reason for hiding this comment

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

@troydai I added this back in. Looks like it was missed from the prev. vcr_test_base

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.

LGTM.

Add CI

Re-record tests

Use actual value in checkers
Copy link
Member

@digimaun digimaun left a comment

Choose a reason for hiding this comment

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

Looks good.



@Completer
def get_device_id_completion_list(cmd, prefix, namespace): # pylint: disable=unused-argument
Copy link
Member

Choose a reason for hiding this comment

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

I know this device completer was here before...but I'd like to note that it isn't that valuable. The device list endpoint returns up to 1000 devices, and you could have a million+ devices on an IoT Hub.

Copy link
Member

Choose a reason for hiding this comment

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

Device CRUD commands would be deprecated in favor of IoT extension equivalents anyway.

@tjprescott tjprescott merged commit 3b64584 into Azure:KnackConversion Dec 13, 2017
@derekbekoe derekbekoe deleted the knack-iot branch December 13, 2017 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants