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

Prompt to update PowerShell version #2105

Merged
merged 8 commits into from
Sep 16, 2019

Conversation

TylerLeonhardt
Copy link
Member

@TylerLeonhardt TylerLeonhardt commented Jul 27, 2019

PR Summary

This adds a little dialog that informs you that you can update your version of PowerShell.

It will update:

6.0 - 6.2.1 -> 6.2.2
any preview version -> 7.0.0-preview.2

here's an example:
image

It updates via homebrew on macOS and through the MSI (interactively) on Windows.

Linux is not supported.

This won't "update" Windows PowerShell to PowerShell 6+ yet as it's a trickier situation.

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets.
Please mark anything not applicable to this PR NA.

  • PR has a meaningful title
  • Summarized changes
  • PR has tests
  • This PR is ready to merge and is not work in progress
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready

Copy link
Contributor

@bergmeister bergmeister left a comment

Choose a reason for hiding this comment

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

Good idea and relatively simple implementation (+1) for such a nice functionality. I think it needs some tweaks but otherwise looks good, don't be put off by the amount of comments.


const PowerShellGitHubReleasesUrl =
"https://api.github.com/repos/PowerShell/PowerShell/releases/latest";
const PowerShellGitHubRPrereleasesUrl =
Copy link
Contributor

Choose a reason for hiding this comment

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

PowerShellGitHubRPrereleasesUrl --> PowerShellGitHubPrereleasesUrl ?

src/features/UpdatePowerShell.ts Outdated Show resolved Hide resolved
src/features/UpdatePowerShell.ts Outdated Show resolved Hide resolved
src/features/UpdatePowerShell.ts Outdated Show resolved Hide resolved
src/session.ts Outdated Show resolved Hide resolved
src/session.ts Show resolved Hide resolved
src/session.ts Outdated Show resolved Hide resolved
src/session.ts Show resolved Hide resolved
@GHRoss
Copy link

GHRoss commented Jul 27, 2019

I really like this.

My only concern (running macOS) is that the user might not have Brew installed, is there a check in here for that?

A quick Google returned this, which might help? https://stackoverflow.com/questions/21577968/how-to-tell-if-homebrew-is-installed-on-mac-os-x

src/features/UpdatePowerShell.ts Outdated Show resolved Hide resolved
src/features/UpdatePowerShell.ts Outdated Show resolved Hide resolved
src/features/UpdatePowerShell.ts Outdated Show resolved Hide resolved
src/features/UpdatePowerShell.ts Outdated Show resolved Hide resolved
src/features/UpdatePowerShell.ts Outdated Show resolved Hide resolved
src/session.ts Show resolved Hide resolved
@TylerLeonhardt
Copy link
Member Author

@GHRoss Good suggestion! Since we're in typescript though, we'd want to use a node.js way of finding out if we have brew which sadly would require yet another package to be brought in.

Considering we'd have to do this, or something less than ideal like execute which ourselves and parse the response, and since PowerShell's recommended install instructions say to use Homebrew, I think we can close to assume it's available...

That said, as a compromise, I'll have the dialog explicitly say it'll use brew:

image

@TylerLeonhardt
Copy link
Member Author

p.s. @rjmholt ...

image

better "never" text?

@TylerLeonhardt
Copy link
Member Author

I think I've addressed all feedback here

TylerLeonhardt and others added 2 commits August 2, 2019 16:40
Co-Authored-By: Christoph Bergmeister [MVP] <[email protected]>
Co-Authored-By: Christoph Bergmeister [MVP] <[email protected]>
src/features/UpdatePowerShell.ts Outdated Show resolved Hide resolved
src/features/UpdatePowerShell.ts Outdated Show resolved Hide resolved
@TylerLeonhardt
Copy link
Member Author

@bergmeister I've addressed all your feedback - I'm going to merge this in today or tomorrow for a release unless you have any more feedback

@TylerLeonhardt
Copy link
Member Author

I will try to backport this to legacy but if it doesn't go well I'm ok keeping this in Preview

@TylerLeonhardt TylerLeonhardt merged commit 66a39de into PowerShell:master Sep 16, 2019
@TylerLeonhardt TylerLeonhardt deleted the update-powershell branch September 16, 2019 19:21
TylerLeonhardt added a commit to TylerLeonhardt/vscode-powershell that referenced this pull request Sep 16, 2019
* prompt to update PowerShell version

* address all feedback

* say homebrew is used

* Feedback

Co-Authored-By: Christoph Bergmeister [MVP] <[email protected]>

* Feedback

Co-Authored-By: Christoph Bergmeister [MVP] <[email protected]>

* PR feedback

* use correct url from github api
TylerLeonhardt added a commit that referenced this pull request Sep 17, 2019
* prompt to update PowerShell version

* address all feedback

* say homebrew is used

* Feedback

Co-Authored-By: Christoph Bergmeister [MVP] <[email protected]>

* Feedback

Co-Authored-By: Christoph Bergmeister [MVP] <[email protected]>

* PR feedback

* use correct url from github api
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.

4 participants