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

Update Windows default shell #1558

Merged
merged 11 commits into from
Jan 8, 2021
Merged

Update Windows default shell #1558

merged 11 commits into from
Jan 8, 2021

Conversation

al-cheb
Copy link
Contributor

@al-cheb al-cheb commented Nov 23, 2020

Why:

Windows default shell is incorrect.

jobs:
  build:
    runs-on: ${{ matrix.os }}
    strategy:
      matrix:
       os: [windows-2016, windows-2019]
    steps:
    - name: Default shell ${{matrix.os}}
      run: $PSVersionTable.PSEdition
    - name: PowerShell Desktop ${{matrix.os}}
      shell: powershell
      run: $PSVersionTable.PSEdition

Windows Server 2016:
image

Windows Server 2019:
image

What's being changed:

Set the default shell is pwsh for Windows.

Check off the following:

  • All of the tests are passing.
  • I have reviewed my changes in staging. (look for the deploy-to-heroku link in your pull request, then click View deployment)
  • For content changes, I have reviewed the localization checklist
  • For content changes, I have reviewed the Content style guide for GitHub Docs.

A rendered preview of these changes is available here: https://docs-1558--al-chebwindows-defa.herokuapp.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#using-a-specific-shell

@welcome
Copy link

welcome bot commented Nov 23, 2020

Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

@janiceilene
Copy link
Contributor

@al-cheb Thanks so much for opening a PR! I'll get this triaged for review 🌟

@janiceilene janiceilene added actions This issue or pull request should be reviewed by the docs actions team content This issue or pull request belongs to the Docs Content team ecosystem This issue or pull request should be reviewed by the Docs Ecosystem team labels Nov 23, 2020
@shati-patel
Copy link
Contributor

(For information: I just updated this PR from main to fix a merge conflict that came from #1389. I'll let someone from the Ecosystem docs team take a look at the actual changes, since they're more familiar with these topics!)

@github-actions
Copy link
Contributor

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

@github-actions github-actions bot added the stale There is no recent activity on this issue or pull request label Dec 10, 2020
@janiceilene
Copy link
Contributor

Thanks for your patience! Our small team is working our way through reviewing all of the amazing contributions ✨

@janiceilene janiceilene removed the stale There is no recent activity on this issue or pull request label Dec 10, 2020
@martin389 martin389 self-assigned this Dec 15, 2020
@martin389
Copy link
Contributor

@martin389
Copy link
Contributor

Status update: Checking these changes with our support folks

Copy link

@moneyglitxh moneyglitxh left a comment

Choose a reason for hiding this comment

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

?

@github-actions
Copy link
Contributor

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

@github-actions github-actions bot added the stale There is no recent activity on this issue or pull request label Dec 29, 2020
@al-cheb
Copy link
Contributor Author

al-cheb commented Dec 29, 2020

@martin389, Thank you.

@github-actions github-actions bot removed the stale There is no recent activity on this issue or pull request label Dec 30, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2021

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

@github-actions github-actions bot added the stale There is no recent activity on this issue or pull request label Jan 7, 2021
@martin389 martin389 removed the stale There is no recent activity on this issue or pull request label Jan 8, 2021
@martin389
Copy link
Contributor

Status update: Following up with support folks ✨

@@ -646,7 +646,8 @@ You can override the default shell settings in the runner's operating system usi
| All | `python` | Executes the python command. | `python {0}` |
| Linux / macOS | `sh` | The fallback behavior for non-Windows platforms if no shell is provided and `bash` is not found in the path. | `sh -e {0}` |
| Windows | `cmd` | {% data variables.product.prodname_dotcom %} appends the extension `.cmd` to your script name and substitutes for `{0}`. | `%ComSpec% /D /E:ON /V:OFF /S /C "CALL "{0}""`. |
| Windows | `powershell` | This is the default shell used on Windows. The Desktop PowerShell. {% data variables.product.prodname_dotcom %} appends the extension `.ps1` to your script name. | `powershell -command ". '{0}'"`. |
| Windows | `pwsh` | This is the default shell used on Windows. The PowerShell Core. {% data variables.product.prodname_dotcom %} appends the extension `.ps1` to your script name. | `pwsh -command ". '{0}'"`. |
Copy link
Contributor

Choose a reason for hiding this comment

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

This is absolutely correct for Hosted runners but I have small note about self-hosted:
If self-hosted Windows runner doesn't have PowerShell Core installed, default shell falls back to PowerShell Desktop.
Related code: https://github.com/actions/runner/blob/main/src/Runner.Worker/Handlers/ScriptHandler.cs#L87

@martin389, do you think it should be mentioned?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @maxim-lobanov, makes sense to me 👍 I'll add a note to mention this.

@martin389 martin389 merged commit 880d86d into github:main Jan 8, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jan 8, 2021

Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours.

@martin389
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actions This issue or pull request should be reviewed by the docs actions team content This issue or pull request belongs to the Docs Content team ecosystem This issue or pull request should be reviewed by the Docs Ecosystem team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants