-
Notifications
You must be signed in to change notification settings - Fork 4k
Enhance Az.Migrate to prep for service release #28020
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
Enhance Az.Migrate to prep for service release #28020
Conversation
Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
To the author of the pull request, |
Hi @minhsuanlee, if it's a breaking change PR, we'll leave this PR open till the breaking change release sprint. |
The modified commands are all in preview which I believe breaking changes are allowed. Let me know if that is not the case. If not, please help me get pass the above failing checks. |
Hi @minhsuanlee, it's not allowed to merge breaking change PRs to a GAed module. Az.Migrate is GAed (major version number is over 1). Thus, breaking changes will only be allowed in major release, and next major release is 15.0.0. |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
For record: |
9aa7397
to
32d17e1
Compare
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
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
This PR enhances the Az.Migrate module by updating help docs, adjusting solution/project references, bumping versioning, and adding new appliance parameters and validations for local server replication.
- Updated ChangeLog and module/project versions for the upcoming 2.8.0 release
- Added
-SourceApplianceName
and-TargetApplianceName
parameters and improved validation logic inNew-AzMigrateLocalServerReplication
- Refactored PowerShell helper and init scripts to use hashtables for parameter invocation and added new input validations
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/Migrate/Migrate/ChangeLog.md | Documented removal/addition of parameters and validations |
src/Migrate/Migrate.sln | Updated project path and GUID, fixed BOM at solution start |
src/Migrate/Migrate.Autorest/docs/README.md | Normalized path separators in docs |
src/Migrate/Migrate.Autorest/docs/New-AzMigrateLocal*.md | Added docs for new appliance-name parameters and removed stale |
src/Migrate/Migrate.Autorest/custom/New-AzMigrateLocal*.ps1 | Updated PreviewMessageAttribute, added params & validations |
src/Migrate/Migrate.Autorest/custom/Initialize-AzMigrateLocalReplicationInfrastructure.ps1 | Enforced provisioning-state checks and refactored calls |
src/Migrate/Migrate.Autorest/custom/Helper/*.ps1 | Enhanced OS/VMware-tools validations and added comments |
src/Migrate/Migrate.Autorest/custom/AzLocalDiskInput.cs | Removed deprecated storageContainerId property |
src/Migrate/Migrate.Autorest/README.md | Bumped auto-rest module-version |
src/Migrate/Migrate.Autorest/Properties/AssemblyInfo.cs | Updated assembly and file versions to 2.8.0 |
Comments suppressed due to low confidence (2)
src/Migrate/Migrate.Autorest/custom/New-AzMigrateLocalServerReplication.ps1:104
- [nitpick] New mandatory parameters lack explicit
ParameterSetName
assignments. Consider specifyingParameterSetName
for clarity and to avoid unintentionally impacting other parameter sets.
[Parameter(Mandatory)]
src/Migrate/Migrate.Autorest/custom/New-AzMigrateLocalDiskMappingObject.ps1:78
- New validation logic for VHD physical sector size was added; please ensure unit tests cover both passing and failing scenarios for this check.
if ($Format -eq "VHD" -and $PhysicalSectorSize -ne 512) {
if ($null -eq $targetDra) | ||
{ | ||
throw $targetDraErrorMessage | ||
} | ||
$targetDra = $targetDras[0] |
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 reassigns $targetDra
from the full $targetDras
list rather than the filtered $targetDra
array. It should be \$targetDra = \$targetDra[0]
to use the previously validated, filtered result.
$targetDra = $targetDras[0] | |
$targetDra = $targetDra[0] |
Copilot uses AI. Check for mistakes.
src/Migrate/Migrate.Autorest/custom/New-AzMigrateLocalServerReplication.ps1
Show resolved
Hide resolved
src/Migrate/Migrate.Autorest/custom/New-AzMigrateLocalServerReplication.ps1
Show resolved
Hide resolved
/azp run azure-powershell - security-tools |
Azure Pipelines successfully started running 1 pipeline(s). |
Description
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.md
and reviewed the following information:ChangeLog.md
file(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
.## Upcoming Release
header in the past tense.ChangeLog.md
if no new release is required, such as fixing test case only.