Added functionality to install the User variant of Stable Edition #2160
Conversation
|
|
|
This fixes #2048 right? |
|
Oh, yeah, I didn't see that it was an issue. I'll edit it to comply with the chosen Option 2 |
There was a problem hiding this comment.
I don't know which one is appropriate to use here either. It looks like all the parameters on the file use double quotes, and all the parameters on the internal function use single quotes. I'd probably leave the call to @rjmholt or @TylerLeonhardt to chime in on which they'd prefer, and if they'd ask it to be changed in this PR or a separate one.
scripts/Install-VSCode.ps1
Outdated
| @@ -132,7 +134,7 @@ param( | |||
| [string]$Architecture = "64-bit", | |||
|
|
|||
| [parameter()] | |||
| [ValidateSet("Stable", "Insider-System", "Insider-User")] | |||
| [ValidateSet("Stable","Stable-User", "Insider-System", "Insider-User")] | |||
There was a problem hiding this comment.
I know you didn't do it, but it bothers me that these are double quotes yet the same ValidateSet on line 204 is single quotes...
And a nit: space between comma and "Stable-User"
There was a problem hiding this comment.
Most of the script uses single quotes. I could change the double quotes when suitable.
scripts/Install-VSCode.ps1
Outdated
| @@ -1,6 +1,6 @@ | |||
| <#PSScriptInfo | |||
|
|
|||
| .VERSION 1.3 | |||
| .VERSION 1.3.1 | |||
There was a problem hiding this comment.
Since you're adding a feature, this would be 1.4.0. Can you update that?
Changed Stable to Stable-System as discussed in Issue PowerShell#2048.
Bumped version to 1.4.1
| $appName = "Visual Studio Code ($Bitness)" | ||
| break | ||
| } | ||
|
|
||
| 'Stable-User' { | ||
| $appName = "Visual Studio Code ($($Architecture) - User)" |
There was a problem hiding this comment.
Any reason you used $Architecture here instead of $Bitness like System?
There was a problem hiding this comment.
No, I did the same as the Insiders Edition - user install.
Co-Authored-By: Tyler James Leonhardt <[email protected]>
| @@ -439,7 +455,7 @@ function Install-VSCodeFromTar { | |||
|
|
|||
| # We need to be running as elevated on *nix | |||
| if (($IsLinux -or $IsMacOS) -and (id -u) -ne 0) { | |||
There was a problem hiding this comment.
It looks like -User editions only apply on Windows (we could install anywhere on Linux from a tar and possibly the same on macOS, but it's likely more complexity than it's worth).
In that case, you might want to add another error message here when the $BuildEdition ends in User but we're on macOS or Linux, to say something like VSCode user installation is only supported on Windows
There was a problem hiding this comment.
There is a similar check at line 226.
It would fire after your suggestion.
Added error message when the $BuildEdition ends in User but we're on macOS or Linux. PowerShell#2160 (comment)
Added error message when the $BuildEdition ends in User but we're on Linux or MacOS
Co-Authored-By: Tyler James Leonhardt <[email protected]>
PR Summary
Added functionality to install the "User Install" variant of Stable Edition in the Install-VSCode.ps1 script. It works in the same way the Insiders Edition "User Install" does (see PR #1477). The deployement website already provides the installers.
Resolves Issue #2048
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.NAWIP:to the beginning of the title and remove the prefix when the PR is ready