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

Silent Staged LS Installer #868

Merged
merged 14 commits into from
Dec 17, 2021
Merged

Silent Staged LS Installer #868

merged 14 commits into from
Dec 17, 2021

Conversation

jpogran
Copy link
Contributor

@jpogran jpogran commented Dec 8, 2021

The terraform-ls installer no longer prompts the user for whether to install an update (or downgrade) and now respects the extensions.autoUpdate and extensions.autoCheckUpdates settings.

If extensions.autoUpdate is set to true, the installer checks for updates on activation and installs a new version if available. If set to false, then it does not check for updates.

When installing an updated terraform-ls version, it installs to a staging directory and continues activation. On restart it will move the terraform-ls binary from the staging location to the extension path and the new version will be used.

If extensions.autoCheckUpdates is set to true, the installer automatically checks for updates every 24 hours. If set to false, it does not check on a schedule.

An update can either be an upgrade or downgrade, depending on the version set in terraform.languageServer.requiredVersion. If latest or no version is specified, the most current terraform-ls version is used. In either an upgrade or a downgrade, the installer will perform the required action without asking the user.

@jpogran jpogran force-pushed the silent_staged_installer branch from 2cf621b to 16c4eb5 Compare December 8, 2021 20:44
@jpogran jpogran changed the title silent staged installer Silent Staged LS Installer Dec 8, 2021
@jpogran jpogran self-assigned this Dec 9, 2021
@jpogran jpogran added the enhancement New feature or request label Dec 9, 2021
@jpogran jpogran linked an issue Dec 9, 2021 that may be closed by this pull request
@jpogran jpogran force-pushed the silent_staged_installer branch from df8c671 to 17c7524 Compare December 9, 2021 20:26
The terraform-ls installer no longer prompts the user for whether to install an update (or downgrade) and now respects the `extensions.autoUpdate` and `extensions.autoCheckUpdates` settings.

If `extensions.autoUpdate` is set to `true`, the installer checks for updates on activation and installs a new version if available. If set to `false`, then it does not check for updates.

When installing an updated terraform-ls version, it installs to a staging directory and continues activation. On restart it will move the terraform-ls binary from the staging location to the extension path and the new version will be used.

If `extensions.autoCheckUpdates` is set to `true`, the installer automatically checks for updates every 24 hours. If set to `false`, it does not check on a schedule.

An update can either be an upgrade or downgrade, depending on the version set in terraform.languageServer.requiredVersion. If latest or no version is specified, the most current terraform-ls version is used. In either an upgrade or a downgrade, the installer will perform the required action without asking the user.
@jpogran jpogran force-pushed the silent_staged_installer branch from 17c7524 to b862d68 Compare December 10, 2021 13:06
@jpogran jpogran requested review from dbanck and radeksimko December 10, 2021 14:07
@jpogran jpogran marked this pull request as ready for review December 10, 2021 14:07
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

Awesome work! It's nice to have the tests covering all essential aspects of this feature.

My comments are only TypeScript / Jest related right now, and I might add more as I understand how everything ties together. I'm open to discussing anything, and we can go through typing the test files together if you want.

Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

I dug into the implementation, and it looks really sound! Just a few minor suggestions. Most of my feedback is related to the style and typing of the unit tests, and we can iterate on that together if you like.

The updater.test.ts is the last file I need to look at, but I thought we might do this in a paring session since it's the biggest of the three tests.

jpogran and others added 4 commits December 13, 2021 19:55
Co-authored-by: Daniel Banck <dbanck@users.noreply.github.com>
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

Awesome job! Thank you for taking the time to update all the tests! 🎉
I think the code base is now much more sound without any ts-nocheck files.

I've added minor style nitpicks. Feel free to merge after addressing those.

@jpogran jpogran merged commit 6ed36c9 into main Dec 17, 2021
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make LS installation silent by default
2 participants