Skip to content

Conversation

adamint
Copy link
Member

@adamint adamint commented Sep 3, 2025

Description

Adds a parameter to the build scripts to build the aspire extension.

Allows acquisition of the CI aspire-extension vsix from the aspire-cli-pr shell script.

Adds the artifact to Packaging.props so it can eventually be downloaded in get-aspire-cli scripts.

@radical is there anything else necessary and does this all look right?? I want to make dogfooding easier and get to the point where we can install the extension from get-aspire-cli.

Planning on following up with changes to get-aspire-cli.

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

@adamint adamint requested review from radical and Copilot September 3, 2025 17:32
Copy link
Contributor

github-actions bot commented Sep 3, 2025

🚀 Dogfood this PR with:

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 11245

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 11245"

@github-actions github-actions bot added the area-engineering-systems infrastructure helix infra engineering repo stuff label Sep 3, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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 adds support for building the VS Code Aspire extension as part of the build process and enables downloading and installing the extension through the get-aspire-cli-pr.sh script. The changes support dogfooding workflows by making it easier to test extension builds from CI.

Key changes:

  • Adds --build-extension parameter to build scripts for building the VS Code extension
  • Extends get-aspire-cli-pr.sh to download and install the Aspire extension from CI artifacts
  • Configures publishing of extension artifacts in the build pipeline

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
extension/package.json Minor formatting cleanup for debug configuration
eng/scripts/get-aspire-cli-pr.sh Adds extension download and installation functionality with new CLI options
eng/common/build.sh Implements extension build functionality for Unix platforms
eng/common/build.ps1 Implements extension build functionality for Windows platforms
eng/build.sh Adds --build-extension parameter handling for the main build script
eng/build.ps1 Adds -buildExtension parameter handling for Windows build script
eng/Publishing.props Configures publishing of .vsix extension files as build artifacts

@adamint
Copy link
Member Author

adamint commented Sep 3, 2025

these CI errors are really confusing me

@@ -21,10 +21,13 @@ jobs:
- name: Checkout code
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2

- name: Install VSCE tool
Copy link
Member

Choose a reason for hiding this comment

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

How much time does this add to the build?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it adds a about a minute. It's not the long pole on the overall build (spread across acquisition and the publish operation itself). It isn't the long pole by any stretch so probably OK.

@radical
Copy link
Member

radical commented Sep 3, 2025

these CI errors are really confusing me

Some of the failures are because the build.cmd script is building the extension when a test project had to be built.

@adamint adamint force-pushed the dev/adamint/build-publish-aspire-extension branch from c070f8d to 7c50ca5 Compare September 10, 2025 05:08
@adamint adamint changed the title Build aspire extension, publish artifacts, and allow acquisition from pr cli shell script Build aspire extension and allow acquisition from pr cli shell script Sep 10, 2025
}

# Function to install VS Code extension
install_aspire_extension() {
Copy link
Member

Choose a reason for hiding this comment

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

Some team members use code-insiders I think it would be good to allow people to target code insiders for specific builds.

@mitchdenny
Copy link
Member

This looks good overall. Good to have this happening as part of the build.

Copy link
Member

@radical radical left a comment

Choose a reason for hiding this comment

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

Just the one comment and +1 for code-insiders. other than LGTM!

@@ -60,6 +64,7 @@ USAGE:
--os OS Override OS detection (win, linux, linux-musl, osx)
--arch ARCH Override architecture detection (x64, x86, arm64)
--hive-only Only install NuGet packages to the hive, skip CLI download
--skip-extension-install Skip VS Code extension download and installation
Copy link
Member

Choose a reason for hiding this comment

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

super nit suggestion - Maybe --skip-extension for fewer characters?

@adamint adamint enabled auto-merge (squash) September 10, 2025 15:29
@adamint adamint merged commit b7914a7 into dotnet:main Sep 10, 2025
305 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 9.5 milestone Sep 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-engineering-systems infrastructure helix infra engineering repo stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants