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

Reset npm lock file version to 1 #780

Merged
merged 1 commit into from
Sep 23, 2021
Merged

Conversation

jpogran
Copy link
Contributor

@jpogran jpogran commented Sep 15, 2021

This sets the npm lockfile version back to 1, which aligns the npm version to the version of node we target and also updates the vscode-languageclient library to the 7.0.0 release, which requires the minimum vs code engine to be 1.52.0.

We currently use node v14.X as the targeted version, which uses npm v6.X. In commit 8bc325c we updated the lockfile to version 2 using npm v7.X. This results in a lockfile mismatch when using node v14.X and npm v6.X to do an npm install. While the npm v7.X lockfile is compatible with npm 6, this does produce a warning that the wrong dependency version may be downloaded. A future commit will handle the migration to npm 7 when we move off of node v14.X.

The vscode-languageclient 7.0.0 release is the current supported release, but we pinned to 7.0.0-next.12. We shouldn't be running on the interim releases, but we were waiting on hashicorp/terraform-ls#608 to merge to support the changed jsonrpc constructs in 7.0.0. Now that that is merged, released, and in the wild for a significant amount of time we can move to the 7.0.0 vscode-languageclient version.

This requires us to increment the minimum supported version of VS Code to 1.52.0. This means any user with a VS Code version lower than 1.52.0 will not see an updated extension in the marketplace, but will still receive updated terraform-ls releases through the automatic updater.

This supersedes #754

@jpogran jpogran self-assigned this Sep 16, 2021
@jpogran jpogran force-pushed the reset-npm-lockfile-version branch 2 times, most recently from 91f91f1 to 256247e Compare September 16, 2021 16:20
@jpogran jpogran marked this pull request as ready for review September 16, 2021 16:23
@radeksimko radeksimko requested a review from a team September 21, 2021 13:58
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

Just for posterity LS 0.21.0 with the mentioned patch hashicorp/terraform-ls#608 was released on 23rd August, so almost 1 month, which seems like a reasonable time period, but I would probably prefer to first release the existing (already merged) PRs and queue up this change for the next release, just to give folks a little more time.

After all the UX without the patch is not great, so if we can even further reduce the chance of people running into it, it would be nice.

package.json Outdated Show resolved Hide resolved
@jpogran
Copy link
Contributor Author

jpogran commented Sep 21, 2021

Putting this as draft until existing release is completed

@jpogran jpogran marked this pull request as draft September 21, 2021 14:32
@radeksimko radeksimko marked this pull request as ready for review September 22, 2021 15:23
@radeksimko
Copy link
Member

2.15.0 is out, so this is now safe for merge, subject to a rebase.

@jpogran jpogran force-pushed the reset-npm-lockfile-version branch from 256247e to a4e8cd6 Compare September 22, 2021 17:43
This updates the vscode-languageclient library to the 7.0.0 release, which requires the minimum vs code engine to be 1.52.0.

The vscode-languageclient 7.0.0 release is the current supported release, but we pinned to 7.0.0-next.12. We shouldn't be running on the interim releases, but we were waiting on hashicorp/terraform-ls#608 to merge to support the changed jsonrpc constructs in 7.0.0. Now that that is merged, released, and in the wild for a significant amount of time we can move to the 7.0.0 vscode-languageclient version.

This requires us to increment the minimum supported version of VS Code to 1.52.0. This means any user with a VS Code version lower than 1.52.0 will not see an updated extension in the marketplace, but will still receive updated terraform-ls releases through the automatic updater.
@jpogran jpogran force-pushed the reset-npm-lockfile-version branch from a4e8cd6 to e619b1f Compare September 22, 2021 17:48
@jpogran
Copy link
Contributor Author

jpogran commented Sep 22, 2021

TODO: fixup commit message

@jpogran jpogran merged commit f656f1c into main Sep 23, 2021
@jpogran jpogran deleted the reset-npm-lockfile-version branch September 23, 2021 14:25
@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 Oct 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants