-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Migrate StorageAction module to tsp #29136
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
base: main
Are you sure you want to change the base?
Conversation
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
|
To the author of the pull request, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. See License.txt in the project root for license information. | ||
| // Changes may cause incorrect behavior and will be lost if the code is regenerated. | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. |
Copilot
AI
Feb 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate copyright notice at lines 1 and 4. Remove the duplicate copyright header to avoid confusion.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
…ageaction-with-tsp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. See License.txt in the project root for license information. | ||
| // Changes may cause incorrect behavior and will be lost if the code is regenerated. | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file contains a duplicate copyright header at the beginning. Lines 1-3 show one copyright header (MIT License), and lines 4-13 show another (Apache License 2.0). This appears to be an error from the code generation process. Only one copyright header should be present at the start of the file.
| - Additional information about change #1 | ||
| --> | ||
| ## Upcoming Release | ||
| * Migrated to typespec |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ChangeLog entry "Migrated to typespec" is too technical for end users. According to repository conventions (guideline 1000003), ChangeLog entries should be written from the user's perspective and explain what changed and how it affects their usage. This is an internal implementation detail that doesn't provide meaningful information to PowerShell users. Consider describing the user-facing impact, such as new features, improvements, or note that this is a non-breaking internal modernization if there are no user-visible changes.
isra-fel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got some questions
| service-dir: "src" | ||
| emitter-output-dir: "{output-dir}/{service-dir}/StorageAction/StorageAction.Autorest" | ||
| clear-output-folder: true | ||
| debug: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we not enable debug in production?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this works similar as autorest --debug and will generate comments of subject/variant/prefix etc,. should be removed in order to reduce size of generated code slightly @Pan-Qi
| @@ -0,0 +1,4 @@ | |||
| "Module","ClassName","Target","Severity","ProblemId","Description","Remediation" | |||
| "Az.StorageAction","Get-AzStorageActionTask","Get-AzStorageActionTask","0","3000","The type of property 'IdentityUserAssignedIdentity' of type 'Microsoft.Azure.PowerShell.Cmdlets.StorageAction.Models.IStorageTask' has changed from 'Microsoft.Azure.PowerShell.Cmdlets.StorageAction.Models.IUserAssignedIdentities' to 'Microsoft.Azure.PowerShell.Cmdlets.StorageAction.Models.IManagedServiceIdentityUserAssignedIdentities'.","Change the type of property 'IdentityUserAssignedIdentity' back to 'Microsoft.Azure.PowerShell.Cmdlets.StorageAction.Models.IUserAssignedIdentities'." | |||
| "Az.StorageAction","New-AzStorageActionTask","New-AzStorageActionTask","0","3000","The type of property 'IdentityUserAssignedIdentity' of type 'Microsoft.Azure.PowerShell.Cmdlets.StorageAction.Models.IStorageTask' has changed from 'Microsoft.Azure.PowerShell.Cmdlets.StorageAction.Models.IUserAssignedIdentities' to 'Microsoft.Azure.PowerShell.Cmdlets.StorageAction.Models.IManagedServiceIdentityUserAssignedIdentities'.","Change the type of property 'IdentityUserAssignedIdentity' back to 'Microsoft.Azure.PowerShell.Cmdlets.StorageAction.Models.IUserAssignedIdentities'." | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this breaking change expected in the migration to TSP, or should we mitigate it instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these breaking changes came from referring common-types v5 -> v6, behavior should be expected
|
|
Description
announcement PR
Mandatory Checklist
Please choose the target release of Azure PowerShell. (⚠️ Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.