Make the Install-VSCode script work on *nix #1597
Conversation
rjmholt
requested review from
TylerLeonhardt,
rkeithhill and
SeeminglyScience
scripts/Install-VSCode.ps1
Outdated
| [switch]$EnableContextMenus | ||
| [switch]$EnableContextMenus, | ||
|
|
||
| [switch]$WhatIf |
There was a problem hiding this comment.
Hmm, this is an anti-pattern in PowerShell. Don't declare this parameter but change the [CmdletBinding()] to [CmdletBinding(SupportsShouldProcess)]. Then, below instead of checking $WhatIf, use if ($PSCmdlet.ShouldProcess(<target>, <action>) { ... }
There was a problem hiding this comment.
Ah I wondered about the right way for this -- I didn't find anything that discussed WhatIf very well. I'll change it 🙂
There was a problem hiding this comment.
Ok @rkeithhill, I've implemented this with ShouldProcess now. Let me know if I'm using it the right way :)
scripts/Install-VSCode.ps1
Outdated
| @@ -462,7 +460,7 @@ try { | |||
|
|
|||
| $installerPath = [System.IO.Path]::Combine($tmpdir, $installerName) | |||
|
|
|||
| if ($onWindows) { | |||
| if ($PSVersionTable.PSVersion.Major -lt 6) { | |||
There was a problem hiding this comment.
BitsTransfer wasn't loading for me 😢
Co-Authored-By: rjmholt <[email protected]>
|
Ok, I've tested it on Windows and Ubuntu with WhatIf (and without -- it's part of an install script I wrote; why I made the changes). Everything seems to be working :) |
PR Summary
Made a few changes so the
Install-VSCode.ps1script supports macOS and various Linux distros as well.It's probably not exhaustive and could do with a bit of testing.
I added a
-WhatIfparameter as well, so you can use that to test and it will do the downloads but won't install anything.PR Checklist
Note: Tick the boxes below that apply to this pull request by putting an
xbetween the square brackets.Please mark anything not applicable to this PR
NA.WIP:to the beginning of the title and remove the prefix when the PR is ready