Skip to content

[IGNORE] Testing pr scripts #10859

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft

[IGNORE] Testing pr scripts #10859

wants to merge 14 commits into from
+5,898 −496

Conversation

radical
Copy link
Member

@radical radical commented Aug 6, 2025

No description provided.

@github-actions github-actions bot added the area-engineering-systems infrastructure helix infra engineering repo stuff label Aug 6, 2025
@radical radical added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) and removed area-engineering-systems infrastructure helix infra engineering repo stuff labels Aug 6, 2025
radical added 12 commits August 7, 2025 18:31
@radical
get-aspire-cli.ps1: switch to comment-based help; remove -Help switch…
f2fb695 
… and custom help handler
@radical
get-aspire-cli.ps1: add helper functions (New-TempDirectory, Remove-T…
b82f77a 
…empDirectory, Get-RuntimeIdentifier) and use them in Install-AspireCli
@radical
get-aspire-cli.ps1: detect archive type by extension (.zip/.tar.gz); …
a007526 
…remove OS param from Expand-AspireCliArchive; default install path to ~/.aspire
@radical
eng/scripts/get-aspire-cli.ps1: Shared block of code
a690f68 
- Group shared helper functions into a clearly marked shared block
- No behavior changes; function signatures preserved
- Align help text default path with ~/.aspire
@radical
fix up
4e51fff
@radical
More shared functions, and a new Get-DEfaultInstallPath
5f9f67d
@radical
ps1: cleanup, and fix up after the shared code move
c45c090
@radical
eng/scripts/get-aspire-cli.sh: add helper functions (get_runtime_iden…
bf08f8f 
…tifier, new_temp_dir, remove_temp_dir) and update usages (RID + temp dir)
@radical
eng/scripts/get-aspire-cli.sh: detect archive type by extension (.zip…
4ef3662 
…/.tar.gz); remove OS dependency in installer; default install path to ~/.aspire; help text aligned
@radical
eng/scripts/get-aspire-cli.sh: Shared block of code
1a73644 
- Group shared helper functions into a clearly marked shared block
- No behavior changes; code only reorganized for clarity
@radical
cleanup
1cc8a9d
@radical
pr scripts with tests
59955e6
@davidfowl
Copy link
Member

Some feedback after using this for a bit:

  1. We need to version the packages (the version should contain the SHA like it does with other prerelease versions). This is currently a deal breaking for dogfooding more than one PR. Lots of caching is happening in toolchain bassed on version and its really hard to cache bust.
  2. The package download is 400MB because we bundle the DCP and dashboard for all operating systems in the same bundle, if the script can download only the required version for the OS it would be ~70 MB.

@radical
Copy link
Member Author

radical commented Aug 9, 2025

Some feedback after using this for a bit:

  1. We need to version the packages (the version should contain the SHA like it does with other prerelease versions). This is currently a deal breaking for dogfooding more than one PR. Lots of caching is happening in toolchain bassed on version and its really hard to cache bust.

Right now it is: 9.5.0-pr.10859 . We can use the sha instead of pr.${prNumber}.${sha}, but wouldn't including the sha` also cause lots of stale hives as one iterates on a PR?

Screenshot 2025-08-09 at 19 41 16
  1. The package download is 400MB because we bundle the DCP and dashboard for all operating systems in the same bundle, if the script can download only the required version for the OS it would be ~70 MB.

We can do this.

@davidfowl
Copy link
Member

Right now it's 9.5.0-ci without a version.

I want a really easy way to know 2 things:

  • I want to make sure I can back track to the commit (e.g. 9.5.0-preview.1.25409.1+4704f1fc6d57cbbbda9c0d2cb514d2a42da4c32f) so it should be in the assmebly info version

Every build needs to be unique, the sha (short version) is a great way to do that. We can include the action id as well.

@davidfowl
Copy link
Member

The hive can be for the PR but the package versions should be unique

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@radical @davidfowl