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

Add some backward compatible code for IsWindows and IsLinux #35

Merged
merged 2 commits into from
Mar 9, 2022

Conversation

harrisonmeister
Copy link
Contributor

We had an internal report about cloning a tentacle instance in the incorrect folder. Presumably, this was on running on Windows PowerShell.

Not a big fan of even having to add this, but it does seem to work.

@CLAassistant
Copy link

CLAassistant commented Mar 9, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@BobJWalker BobJWalker left a comment

Choose a reason for hiding this comment

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

This looks to be a bit overkill. IsWindows and IsLinux was added in PowerShell Core 6. PowerShell 5.1 and lower wouldn't have those variables. But those versions of PS only run on Windows.

If those variables are not set then it is safe to assume they are running on Windows.

@harrisonmeister
Copy link
Contributor Author

This looks to be a bit overkill. IsWindows and IsLinux was added in PowerShell Core 6. PowerShell 5.1 and lower wouldn't have those variables. But those versions of PS only run on Windows.

If those variables are not set then it is safe to assume they are running on Windows.

@BobJWalker - I've removed the code I added , and just switched around the one place where testing for $IsWindows was failing to default to Windows.

Copy link
Contributor

@BobJWalker BobJWalker left a comment

Choose a reason for hiding this comment

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

LGTM

@BobJWalker BobJWalker merged commit 6031fb0 into main Mar 9, 2022
@harrisonmeister harrisonmeister deleted the target-os branch March 9, 2022 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants