Skip to content

Conversation

@kairu-ms
Copy link
Contributor

@kairu-ms kairu-ms commented Mar 22, 2021

Description

Context for the change
AzCommandGroup are using _cli_command, _cli_show_command, _cli_wait_command and _cli_generic_update_command function to register a command into command table. The logic of above functions are not clear, and they are not compatible with Configurational CLI.

What is changed

  • replace function _cli_generic_update_command by class GenericUpdateCommandOperation
  • replace function _cli_wait_command by class WaitCommandOperation
  • replace function _cli_show_command by class ShowCommandOperation
  • deprecate function _cli_command by add_cli_command and its internal functions move to class CommandOperation
  • update generic_update_command, _wait_command, _show_command and _command methods of AzCommandGroup

How to test
This change should be 100% compatible with existing code. So it should pass all of the existing tests.

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.


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

@yonzhan
Copy link
Collaborator

yonzhan commented Mar 22, 2021

Core

@yonzhan yonzhan requested a review from jiasli March 22, 2021 09:04
@yonzhan yonzhan added this to the S185 milestone Mar 22, 2021
@yonzhan yonzhan requested a review from jsntcy March 22, 2021 09:04
@kairu-ms kairu-ms marked this pull request as ready for review March 23, 2021 03:46
@kairu-ms kairu-ms requested a review from houk-ms as a code owner March 23, 2021 03:46
@kairu-ms kairu-ms force-pushed the az-command-group-refactoring branch from 02b67ef to e5eb2dc Compare March 23, 2021 06:19
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's replaced by GenericUpdateCommandOperation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's replaced by WaitCommandOperation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's replaced by ShowCommandOperation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Internal functions are replaced by CommandOperation

@kairu-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@kairu-ms kairu-ms mentioned this pull request Mar 23, 2021
3 tasks
Copy link
Contributor

Choose a reason for hiding this comment

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

the same concern as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same reason as above

@kairu-ms kairu-ms force-pushed the az-command-group-refactoring branch from faab3a1 to 5866b8e Compare March 24, 2021 07:27
@kairu-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Contributor

@Juliehzl Juliehzl left a comment

Choose a reason for hiding this comment

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

LGTM

@yonzhan yonzhan modified the milestones: S185, S186 Apr 10, 2021
@jiasli
Copy link
Member

jiasli commented Apr 12, 2021

This is a rather big PR, please add description to illustrate

  1. Context for the change
  2. What is changed
  3. How to test

@kairu-ms kairu-ms merged commit 1ca05db into Azure:dev Apr 12, 2021
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.

4 participants