Skip to content

Fix positional parameter ordering in validate-typescript-apphost.ps1#1437

Closed
Omnideth with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-code-for-review-comment
Closed

Fix positional parameter ordering in validate-typescript-apphost.ps1#1437
Omnideth with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-code-for-review-comment

Conversation

Copilot AI commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

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 the param(...) block, after $Secrets, preserving the original positional order of all pre-existing parameters.

Before (breaks positional callers):

param(
    [string]$AppHostPath,       # pos 1
    [string]$PackageProjectPath, # pos 2 — NEW, displaces existing params
    [string]$PackageName,        # pos 3 — NEW
    [switch]$UseConfiguredPackages, # pos 4 — NEW
    [string[]]$WaitForResources, # pos 5 — was 2
    ...
)

After (non-breaking):

param(
    [string]$AppHostPath,        # pos 1
    [string[]]$WaitForResources, # pos 2 — unchanged
    ...
    [string[]]$Secrets,          # pos 6 — unchanged
    [string]$PackageProjectPath, # pos 7 — NEW, appended
    [string]$PackageName,        # pos 8 — NEW
    [switch]$UseConfiguredPackages # pos 9 — NEW
)

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
Copilot AI requested a review from Omnideth June 29, 2026 04:22
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.sh | bash -s -- 1437

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.ps1) } 1437"

@Omnideth Omnideth marked this pull request as ready for review June 29, 2026 04:34
Copilot AI review requested due to automatic review settings June 29, 2026 04:34
@Omnideth

Copy link
Copy Markdown
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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 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 $UseConfiguredPackages after the existing $Secrets parameter to preserve the original positional ordering of existing parameters.
  • Updates the $Secrets declaration to include a trailing comma to allow additional parameters to follow.

Comment on lines +18 to +22
[string]$PackageProjectPath,

[string]$PackageName,

[switch]$UseConfiguredPackages
@Omnideth Omnideth closed this Jun 29, 2026
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.

3 participants