Skip to content

Preview extension notation & Simple default view for extension list-a…#5882

Merged
derekbekoe merged 4 commits intoAzure:devfrom
derekbekoe:feature/ext-preview-flag
Mar 22, 2018
Merged

Preview extension notation & Simple default view for extension list-a…#5882
derekbekoe merged 4 commits intoAzure:devfrom
derekbekoe:feature/ext-preview-flag

Conversation

@derekbekoe
Copy link
Copy Markdown
Member

Preview extension notation & Simple default view for extension list-available


This checklist is used to make sure that common guidelines for a pull request are followed.

  • 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).

  • I adhere to the Command Guidelines.

@promptws
Copy link
Copy Markdown

View a preview at https://prompt.ws/r/Azure/azure-cli/5882
This is an experimental preview for @microsoft users.

@derekbekoe derekbekoe requested review from troydai and williexu March 21, 2018 23:02
@derekbekoe derekbekoe added this to the Sprint 34 milestone Mar 21, 2018
@derekbekoe derekbekoe added the Extensions `az extension` commands or extension infrastructure label Mar 21, 2018
@derekbekoe
Copy link
Copy Markdown
Member Author

Some screenshots:

screen shot 2018-03-21 at 4 00 24 pm
screen shot 2018-03-21 at 3 48 18 pm
screen shot 2018-03-21 at 3 48 35 pm
screen shot 2018-03-21 at 3 48 44 pm

@tjprescott
Copy link
Copy Markdown
Member

I'll let Troy and Willie comment on the code itself, but from the screenshot my only feedback is it might make sense to put the Preview and Installed columns to the left of summary (closer to the name) because there can be a lot of whitespace in the summary and it can be tricky to follow across the row to see which extension that random "True" floating in space refers to.

…vailable

- Show message for extensions marked as preview on -h
@derekbekoe derekbekoe force-pushed the feature/ext-preview-flag branch from 2b1e86c to b04089d Compare March 22, 2018 19:02
Copy link
Copy Markdown
Contributor

@williexu williexu 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 overall, one question

'preview': latest['metadata'].get(EXT_METADATA_ISPREVIEW, False),
'installed': installed
})
return results
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we have a test for this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I do have some tests but not for these changes...
I will take a look at adding tests... 👍

@derekbekoe
Copy link
Copy Markdown
Member Author

@williexu can you take another look?

Copy link
Copy Markdown
Contributor

@williexu williexu left a comment

Choose a reason for hiding this comment

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

Added testing looks good

@derekbekoe derekbekoe merged commit e4339a7 into Azure:dev Mar 22, 2018
@derekbekoe derekbekoe deleted the feature/ext-preview-flag branch March 22, 2018 22:50
@haroldrandom haroldrandom added the Extensions `az extension` commands or extension infrastructure label Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Extensions `az extension` commands or extension infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Simple default view for extension list-available

6 participants