Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

set default shell to powershell in windows #135

Merged
merged 3 commits into from
Oct 21, 2019

Conversation

thboop
Copy link
Collaborator

@thboop thboop commented Oct 18, 2019

Chose to not do this via a feature flag as we are rolling out right after another runner so the set of changes should be minimal

https://github.com/github/pe-actions-runtime/issues/84

BREAKING CHANGE we wish to roll this out on 10/23 after ChrisPat has given sufficient notice to customers

@thboop thboop requested review from juliobbv and ericsciple October 18, 2019 17:51
@ericsciple
Copy link
Collaborator

@bryanmacfarlane @chrispat do we need a feature flag so we can quickly mitigate specific accounts? or is the mitigation to tell customers to add shell: cmd?

@ericsciple
Copy link
Collaborator

make sure we get the desired behavior where $LASTEXITCODE bubbles failure

@thboop
Copy link
Collaborator Author

thboop commented Oct 21, 2019

@bryanmacfarlane @chrispat do we need a feature flag so we can quickly mitigate specific accounts? or is the mitigation to tell customers to add shell: cmd?

I spoke to Bryan about this and we don't think this is needed because we are doing this as an independent runner release so we can roll back if needed. We don't really envision toggling this feature flag as once it sits in production for a bit toggling the flag would break users dependent on the functionality.

@chrispat
Copy link
Member

I agree that we don't really want to toggle this one.

Copy link
Contributor

@dakale dakale left a comment

Choose a reason for hiding this comment

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

LGTM

@thboop
Copy link
Collaborator Author

thboop commented Oct 21, 2019

make sure we get the desired behavior where $LASTEXITCODE bubbles failure

This correctly fails builds when run: exit 1 is invoked. the code does go down the correct path as well to set this behavior

@thboop thboop merged commit 9d7b0a2 into master Oct 21, 2019
@thboop thboop deleted the users/thboop/changeWindowsdefaultToPowershell branch October 22, 2019 17:15
AdamOlech pushed a commit to antmicro/runner that referenced this pull request Jan 28, 2021
* set default shell to powershell in windows

* Update error checking

* Update error message
volker-raschek pushed a commit to dedalus-cis4u/github-runner-role that referenced this pull request Jul 25, 2022
volker-raschek pushed a commit to dedalus-cis4u/github-runner-role that referenced this pull request Jul 25, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants