Skip to content

Remove languageserver-types dependency in favor of -protocol#10500

Merged
colin-grant-work merged 4 commits intomasterfrom
dependencies/languageserver-protocol-not-types
Dec 15, 2021
Merged

Remove languageserver-types dependency in favor of -protocol#10500
colin-grant-work merged 4 commits intomasterfrom
dependencies/languageserver-protocol-not-types

Conversation

@colin-grant-work
Copy link
Contributor

What it does

The vscode-languageserver-types package is part of and reexported by the vscode-languageserver-protocol package. There's no good reason to have both, and having both separately could introduce dependency management issues, so this replaces all references to vscode-languageserver-types with vscode-languageserver-protocol. This will substantially reduce the diff of #10265.

How to test

  1. There should be no changes in application behavior - this was a pure search-and-replace operation.

Review checklist

Reminder for reviewers

@colin-grant-work colin-grant-work added the quality issues related to code and application quality label Dec 3, 2021
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

Looks good. However, I found some references to vscode-languageserver-types you seemed to have overlooked:

Also, there is still some reference to it in our core/package.json. I thought the idea was to not depend on a specific version of vscode-languageserver-types, so I'd remove it there as well.

@colin-grant-work
Copy link
Contributor Author

Looks good. However, I found some references to vscode-languageserver-types you seemed to have overlooked:

Also, there is still some reference to it in our core/package.json. I thought the idea was to not depend on a specific version of vscode-languageserver-types, so I'd remove it there as well.

Good catches - thanks. I was so concerned about the shared dependency that that's what I searched for, ignoring core.

@colin-grant-work colin-grant-work force-pushed the dependencies/languageserver-protocol-not-types branch from 9b48a1f to 7def431 Compare December 6, 2021 16:08
@vince-fugnitto vince-fugnitto added the dependencies issues that plan to update dependencies label Dec 6, 2021
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

@colin-grant-work do you mind adding a breaking change entry for anyone using the vscode-languageserver-types import as it is no longer re-exported (shared) from core? Other than that the changes look good to me, and I confirmed there are no uses of vscode-languageserver-types directly anymore in the codebase.

@colin-grant-work colin-grant-work force-pushed the dependencies/languageserver-protocol-not-types branch from 7def431 to d424a76 Compare December 14, 2021 17:43
@colin-grant-work
Copy link
Contributor Author

@vince-fugnitto, I've added an entry to the changelog, but under 1.22 - no need to rush to get it in before the release on Thursday.

@colin-grant-work colin-grant-work force-pushed the dependencies/languageserver-protocol-not-types branch from d424a76 to ba9c0d0 Compare December 14, 2021 17:49
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes look good to me 👍

@colin-grant-work colin-grant-work merged commit 6509f69 into master Dec 15, 2021
@colin-grant-work colin-grant-work deleted the dependencies/languageserver-protocol-not-types branch December 15, 2021 15:24
@github-actions github-actions bot added this to the 1.21.0 milestone Dec 15, 2021
@vince-fugnitto vince-fugnitto mentioned this pull request Feb 7, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies issues that plan to update dependencies quality issues related to code and application quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants