Skip to content
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.

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

Standardize our Powershell code #72423

Merged
merged 2 commits into from
Mar 8, 2024
Merged

Standardize our Powershell code #72423

merged 2 commits into from
Mar 8, 2024

Conversation

jaredpar
Copy link
Member

@jaredpar jaredpar commented Mar 6, 2024

In #72341 I made an attempt to standardize our Powershell on pwsh. Unfortunately attempting to get that PR green revealed a number of issues and in following up on those issues with core-eng I came to the conclusion that it's not possible to move fully to pwsh at this time. The issue is that there are downstream consumers of our repo that use Powershell and they have no plans to move hence moving here will just end up breaking source build, unified build, etc ...

As a result I went through our code and removed all of the existing pwsh usage. I also took the time to update our Powershell Guidelines document to enforce using Powershell as well as a few other items. Then I went through our Powershell scripts and ensured they conformed to these guidelines.

The one place that remains able to usee pwsh is our VSCode supporting scripts as that is not part of our build infra.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 6, 2024
@jaredpar jaredpar marked this pull request as ready for review March 7, 2024 16:43
@jaredpar jaredpar requested a review from a team as a code owner March 7, 2024 16:43
@jaredpar
Copy link
Member Author

jaredpar commented Mar 7, 2024

@dotnet/roslyn-infrastructure PTAL

docs/contributing/Powershell Guidelines.md Outdated Show resolved Hide resolved
# DO
Exec-Command $command $args
& eng/make-bootstrap.ps1
Test-LastExitCode
Copy link
Member

@jjonescz jjonescz Mar 8, 2024

Choose a reason for hiding this comment

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

What was wrong with Exec-Script?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is very difficult to pass switch based values between scripts with Exec-Script and Powershell. For example the following is tricky:

Exec-Script "eng/build.ps1" "-ci:$ci" 

That turns into -ci:true which isn't a valid Powershell calling syntax.

Co-authored-by: Jan Jones <jan.jones.cz@gmail.com>
@jaredpar jaredpar enabled auto-merge (squash) March 8, 2024 15:33
@jaredpar
Copy link
Member Author

jaredpar commented Mar 8, 2024

Not sure why AzDO got disconnected from the PR but here is the build with all passing tests

https://dnceng-public.visualstudio.com/public/_build/results?buildId=593595&view=results

@jaredpar jaredpar disabled auto-merge March 8, 2024 18:26
@jaredpar jaredpar merged commit c625ed1 into dotnet:main Mar 8, 2024
2 checks passed
@jaredpar jaredpar deleted the powershell branch March 8, 2024 18:26
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Mar 8, 2024
@RikkiGibson RikkiGibson modified the milestones: Next, 17.10 P3 Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants