Closed
Fix positional parameter ordering in validate-typescript-apphost.ps1#1437
Conversation
Co-authored-by: Omnideth <3496652+Omnideth@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix code for review comment in PR #1436
Fix positional parameter ordering in validate-typescript-apphost.ps1
Jun 29, 2026
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.sh | bash -s -- 1437Or
iex "& { $(irm https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.ps1) } 1437" |
Omnideth
approved these changes
Jun 29, 2026
Contributor
|
This was the first time I tried fixing it in the github web interface with copilot (mainly just to get experience trying it this way.) I prefer doing it the other way, but I believe this is fine to address the review comment left by the copilot code review on the original PR. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts the PowerShell parameter list in the TypeScript AppHost validation harness to avoid silent breaking changes for callers that rely on positional parameter binding by appending newly introduced optional parameters to the end of the param(...) block.
Changes:
- Appends
$PackageProjectPath,$PackageName, and$UseConfiguredPackagesafter the existing$Secretsparameter to preserve the original positional ordering of existing parameters. - Updates the
$Secretsdeclaration to include a trailing comma to allow additional parameters to follow.
Comment on lines
+18
to
+22
| [string]$PackageProjectPath, | ||
|
|
||
| [string]$PackageName, | ||
|
|
||
| [switch]$UseConfiguredPackages |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding new optional parameters in the middle of a PowerShell
param(...)block shifts the positional indices of all subsequent parameters, which is a silent breaking change for callers using positional binding.Changes
eng/testing/validate-typescript-apphost.ps1: Moved the three new optional parameters ($PackageProjectPath,$PackageName,$UseConfiguredPackages) to the end of theparam(...)block, after$Secrets, preserving the original positional order of all pre-existing parameters.Before (breaks positional callers):
After (non-breaking):