Skip to content

Conversation

@andyleejordan
Copy link
Member

PR Summary

This removes the -Daily flag from the install-powershell.ps1 script as daily builds have not been posted in a year, and @adityapatwardhan says there are no current plans to resume doing so.

Also updates the script to install the Preview to ~/.powershell-preview like the daily was.

PR Context

I use this for testing PowerShell Editor Services and PSScriptAnalyzer in CI against upcoming builds of PowerShell to find bugs before they make it to release.

PR Checklist

@andyleejordan
Copy link
Member Author

Required for PowerShell/PowerShellEditorServices#2225.

@andyleejordan andyleejordan force-pushed the install-powershell branch 3 times, most recently from d214f23 to 31bb622 Compare March 5, 2025 01:17
@andyleejordan
Copy link
Member Author

The bullet style was changed in order to satisfy the Markdown linter 🙃

@andyleejordan
Copy link
Member Author

Also required for PowerShell/PSScriptAnalyzer#2070.

@andyleejordan andyleejordan enabled auto-merge (squash) March 5, 2025 01:31
@iSazonov
Copy link
Collaborator

iSazonov commented Mar 5, 2025

This removes the -Daily flag from the install-powershell.ps1 script as daily builds have not been posted in a year, and @adityapatwardhan says there are no current plans to resume doing so.

We have a request to have daily build #24566

In my understanding, daily builds are the only way for partner teams to continuously test their products.
This means that we will learn about regression only after the next preview release and only if they use it for tests.
Obviously, it's better for them to test on a daily basis than to patch up problems a year later in an emergency but we even deprived them of the opportunity.

@JustinGrote
Copy link
Contributor

JustinGrote commented Mar 6, 2025

@andyleejordan should add a parameter alias to "Daily" to not break existing scripts. I'd say this merge as-is after that, but then a follow-on should replace daily with "CIBuild" or "GHBuild" to fetch the latest commit azdo build.

@iSazonov per my comments on that, a "latest main commit build" is a better approach than a daily since tests are enforced for main commits.

@iSazonov
Copy link
Collaborator

iSazonov commented Mar 7, 2025

@iSazonov per my comments on that, a "latest main commit build" is a better approach than a daily since tests are enforced for main commits.

It depends on the needs of that project. If they want, they can track each pwsh PR and upload artifacts from it.
I would speculate that for most projects this is too expensive and doesn't make sense. Most likely, they run a full set of functional tests once a day or less.

@JustinGrote
Copy link
Contributor

JustinGrote commented Mar 7, 2025

@iSazonov per my comments on that, a "latest main commit build" is a better approach than a daily since tests are enforced for main commits.

It depends on the needs of that project. If they want, they can track each pwsh PR and upload artifacts from it. I would speculate that for most projects this is too expensive and doesn't make sense. Most likely, they run a full set of functional tests once a day or less.

Then they can still do that, and they'll get whatever the latest PR branch is available when they schedule their job to run on a daily basis. Solves both problems :)

Dailies are an outdated development concept from the days where most work happened "during the day" and commit checks were rare. Since PowerShell has extensive gatekeeping on the main branch, pretty much every main commit can probably be considered to be fairly stable and unbroken at least for the vast majority of test cases, so it makes sense to provide an easy way to consume these artifacts that are already being built.

@andyleejordan
Copy link
Member Author

@andyleejordan should add a parameter alias to "Daily" to not break existing scripts. I'd say this merge as-is after that, but then a follow-on should replace daily with "CIBuild" or "GHBuild" to fetch the latest commit azdo build.

Yeah, I'm wondering if the nicer thing to do is a) warn and just install the preview instead, or b) warn and exit with error. Right now it fails and that hasn't been breaking anyone so B is an option, but A is an option, though it's kind of a lie.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Mar 19, 2025
@andyleejordan
Copy link
Member Author

Yeah, I'm wondering if the nicer thing to do is a) warn and just install the preview instead, or b) warn and exit with error. Right now it fails and that hasn't been breaking anyone so B is an option, but A is an option, though it's kind of a lie.

@TravisEz13 what would you prefer here and what do we want to communicate about the future of the daily build here?

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Mar 19, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Mar 26, 2025
@andyleejordan
Copy link
Member Author

@TravisEz13 waiting on a decision from you / maintainers ☺️

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Mar 26, 2025
@andyleejordan
Copy link
Member Author

Also noting that we'll want to cleanup:

Also install the Preview to `~/.powershell-preview` like the daily was.
@andyleejordan
Copy link
Member Author

I never got an answer so I'm going with option A.

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Apr 4, 2025
@andyleejordan
Copy link
Member Author

@adityapatwardhan can you review this please?

@microsoft-github-policy-service
Copy link
Contributor

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

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

Labels

Review - Needed The PR is being reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants