Skip to content

Breaking Change - Removed ApiVersion parameter from Az.Resources deployments cmdlets and changed the output type of Get-AzResourceGroupDeploymentOperations#13112

Merged
isra-fel merged 26 commits intoAzure:masterfrom
Xynoclafe:xynoclafe/apiversion
Oct 16, 2020
Merged

Breaking Change - Removed ApiVersion parameter from Az.Resources deployments cmdlets and changed the output type of Get-AzResourceGroupDeploymentOperations#13112
isra-fel merged 26 commits intoAzure:masterfrom
Xynoclafe:xynoclafe/apiversion

Conversation

@Xynoclafe
Copy link
Copy Markdown
Contributor

@Xynoclafe Xynoclafe commented Sep 29, 2020 •
edited
Loading

Description

Removed ApiVersion parameter from Az.Resources deployments cmdlets and changed the output type of Get-AzResourceGroupDeploymentOperations

This is a breaking change.
Design review - https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/707

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
    • the markdown help files have been regenerated using the commands listed here

Xynoclafe added 20 commits June 9, 2020 20:16
@Xynoclafe
Remove ApiVersion Parameter from deployments
a009dcd
@Xynoclafe
update changelog
38585ee
@Xynoclafe
Merge branch 'master' of https://github.com/Azure/azure-powershell in…
fcb6129 
…to xynoclafe/apiversion
@Xynoclafe
Delete ApiVersion param
8568c38
@Xynoclafe
Switch to SDK use in ResourceGroup Deployments cmdlets
603df06
@Xynoclafe
Include review fixes
f8f9c77
@Xynoclafe
Merge branch 'master' into xynoclafe/deploymentsSDKfix
d00f0c4
@Xynoclafe
Changed help file to reflect new output type
77798dc
@Xynoclafe
Merge branch 'master' of https://github.com/Azure/azure-powershell in…
4428f27 
…to xynoclafe/deploymentsSDKfix
@Xynoclafe
Rollback breaking changes from PR Azure#12141
e8556bd
@Xynoclafe
Merge branch 'xynoclafe/deploymentsSDKfix' into xynoclafe/apiversion
a2ff0c1
@Xynoclafe
Merge branch 'Az.Resources-preview' of https://github.com/Azure/azure…
07f418d 
…-powershell into xynoclafe/apiversion
@Xynoclafe
Change Get-AzResourceGroupDeploymentOperation implementation to use SDK
eefb5dc
@Xynoclafe
Update help
4a530a2
@Xynoclafe
Rollback changes to help files of untouched cmdlets
4f7421b
@Xynoclafe
Suppress breaking change issues
c65f0b8
@Xynoclafe
remove subscriptionId param
4fd788d
@Xynoclafe
Update breakingchanges csv
2e1f949
@Xynoclafe
Suppress breaking changes
88be509
@Xynoclafe
Merge branch 'master' of https://github.com/Azure/azure-powershell in…
6c9887e 
…to xynoclafe/apiversion
@Xynoclafe Xynoclafe requested a review from filizt September 29, 2020 23:21
@Xynoclafe Xynoclafe changed the title Xynoclafe/apiversion Breaking Change - Removed ApiVersion parameter from Az.Resources deployments cmdlets and changed the output type of Get-AzResourceGroupDeploymentOperations Sep 29, 2020
@filizt
Copy link
Copy Markdown
Contributor

filizt commented Sep 30, 2020 

public abstract class ResourceManagerCmdletBaseWithAPiVersion : ResourceManagerCmdletBase

Could you please fix the casing of "APi" in the class name?

Refers to: src/Resources/ResourceManager/Implementation/CmdletBase/ResourceManagerCmdletBaseWithAPiVersion.cs:21 in 6c9887e. [](commit_id = 6c9887e, deletion_comment = False)

filizt
filizt reviewed Sep 30, 2020
View reviewed changes
src/Resources/Resources/ChangeLog.md Outdated
-->
## Upcoming Release
* Updated `Get-AzResourceGroupDeploymentOperation`to use the SDK.
* Remove ApiVersion parameter
Copy link
Copy Markdown
Contributor

@filizt filizt Sep 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think it's a worthy effort to make the change longs a bit verbose to make things clear. We can add something like "Remove ApiVersion parameter from Deployment cmdlets" (if the cmdlet list is not too long, maybe add them too?)

Copy link
Copy Markdown
Contributor Author

@Xynoclafe Xynoclafe Sep 30, 2020

Choose a reason for hiding this comment

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

There are quite a few cmdlets (mentioned in the design review), so I don't think adding them to the changelog is practical. But I'll make it a little more verbose.

@filizt
Copy link
Copy Markdown
Contributor

filizt commented Sep 30, 2020

Could you also add the link to the design discussion in the description section?

@isra-fel isra-fel self-assigned this Sep 30, 2020
@Xynoclafe
Copy link
Copy Markdown
Contributor Author

Xynoclafe commented Oct 2, 2020

@isra-fel Can you please review this PR?

@isra-fel
Copy link
Copy Markdown
Member

isra-fel commented Oct 9, 2020

/azp run azure-powershell - windows-powershell

@azure-pipelines
Copy link
Copy Markdown
Contributor

azure-pipelines bot commented Oct 9, 2020

No pipelines are associated with this pull request.

isra-fel
isra-fel previously approved these changes Oct 9, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@isra-fel isra-fel left a comment

Choose a reason for hiding this comment

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

Sorry @Xynoclafe our team are just back from holiday. Anyway your changes LGTM. Thanks

@isra-fel isra-fel added Contains Breaking Change This PR contains breaking change Waiting for CI :shipit: and removed needs-review labels Oct 9, 2020
@isra-fel
Copy link
Copy Markdown
Member

isra-fel commented Oct 9, 2020

/azp run

@azure-pipelines
Copy link
Copy Markdown
Contributor

azure-pipelines bot commented Oct 9, 2020

Azure Pipelines successfully started running 3 pipeline(s).

@Xynoclafe
Copy link
Copy Markdown
Contributor Author

Xynoclafe commented Oct 12, 2020

Hi @isra-fel
Can you review the PR again? Your last commit voided your previous review. And I think we can remove the Waiting for CI label as CI seems to be passing now.

@isra-fel
Copy link
Copy Markdown
Member

isra-fel commented Oct 15, 2020

@Xynoclafe I'm sorry but it seems the merge of #13090 has caused a lot of conflict in the test recording file. Would you please resolve them by merging master and record ResourceGroupDeploymentEndToEnd again?

Thanks in advance!

@isra-fel isra-fel merged commit 1cfab69 into Azure:master Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@isra-fel isra-fel isra-fel approved these changes

+1 more reviewer

@filizt filizt filizt left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@isra-fel isra-fel

Labels

Contains Breaking Change This PR contains breaking change Merge Conflicts Waiting for CI :shipit:

Projects

None yet

Milestone

S177 (2020-10-27) Major Release

Development

Successfully merging this pull request may close these issues.

4 participants

@Xynoclafe @filizt @isra-fel @azuresdkci