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

Rewrite UpdatePowerShell feature #4306

Merged
merged 3 commits into from
Dec 6, 2022
Merged

Rewrite UpdatePowerShell feature #4306

merged 3 commits into from
Dec 6, 2022

Conversation

andyleejordan
Copy link
Member

@andyleejordan andyleejordan commented Dec 5, 2022

It's way better now, relying on GitHub less and instead mimicking what PowerShell itself does, and resolves #3226.

Requires PowerShell/PowerShellEditorServices#1965.

@ghost ghost added Area-Startup Issue-Enhancement A feature request (enhancement). labels Dec 5, 2022
@andyleejordan andyleejordan force-pushed the andschwa/auto-update branch 2 times, most recently from cb304b7 to 2698ac1 Compare December 5, 2022 21:27
@andyleejordan andyleejordan marked this pull request as ready for review December 5, 2022 21:28
@andyleejordan andyleejordan requested a review from a team as a code owner December 5, 2022 21:28
@andyleejordan andyleejordan force-pushed the andschwa/auto-update branch 2 times, most recently from cebad6a to 0a66c89 Compare December 6, 2022 17:11
@andyleejordan
Copy link
Member Author

@SeeminglyScience you can pretty much ignore the change from Logger to ILogger, that was so I could instantiate a TestLogger class that doesn't do anything and pass it along to the constructor of UpdatePowerShell (and probably other things when we get more robust client unit tests).

For Linux, macOS, and unsupported (mostly Arm64) architectures on Windows, the update prompt now opens the user's configured URL handler to the GitHub release for the new release. 🥳 While we never supported auto-updating on Linux, this does mean I took out the brew cask update script for macOS. It can work, but it should only be run if we can assert first that the pwsh we're updating was installed by brew, which we weren't doing and isn't always the case. For instance, I install straight from the .pkg installer and so had turned off the update prompt. Also, we should do it using vscode.env.ShellExecution instead of relying on the language server to run a script (to update its own executable). So I've punted that work because it's complicated and requires an update to do right.

@andyleejordan andyleejordan force-pushed the andschwa/auto-update branch 2 times, most recently from ecfc767 to 156372c Compare December 6, 2022 20:47
@andyleejordan andyleejordan requested a review from a team as a code owner December 6, 2022 20:47
Better support, with more tests!
And remove an unused `IRunspaceDetails` interface.
Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

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

LGTM!

@andyleejordan andyleejordan merged commit ba430f6 into main Dec 6, 2022
@andyleejordan andyleejordan deleted the andschwa/auto-update branch December 6, 2022 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Startup Issue-Enhancement A feature request (enhancement).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PowerShell update checker ignores POWERSHELL_UPDATECHECK environment variable
2 participants