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

Stick to vscode-languageserver@6.0.0-next.1 #6607

Merged
merged 1 commit into from
Nov 25, 2019
Merged

Stick to vscode-languageserver@6.0.0-next.1 #6607

merged 1 commit into from
Nov 25, 2019

Conversation

tolusha
Copy link
Contributor

@tolusha tolusha commented Nov 22, 2019

Signed-off-by: Anatoliy Bazko abazko@redhat.com

Reference issue

#6542

What it does

Stick to vscode-languageserver@6.0.0-next.1 for vscode-json-languageserver

How to test

  1. Remove node_modules
  2. Build the project
  3. Open tasks.json and check if autocompletion works

Review checklist

Reminder for reviewers

@akosyakov
Copy link
Member

@marechal-p Would next yarn upgrade rewrite yarn.lock for this dependency? With yarn resolutions, it should not, right?

@akosyakov akosyakov added the dependencies pull requests that update a dependency file label Nov 22, 2019
@tsmaeder
Copy link
Contributor

tsmaeder commented Nov 22, 2019

Could a workaround be to explicitly require vscode-languageserver:6.0.0-next.1 in packages/json/package.json? That way, it is likely that we change the required version when we update the langugage server version.
In case the "resolutions" fix suggested by @akosyakov does not work.

@akosyakov
Copy link
Member

@tsmaeder it should be another way around. If pinning is possible in package.json then it is the best way. It will work for all adopters. But as far as I understood it is not, so then yarn resolutions is cleaner than changing yarn lock file. Yarn lock file will be rewritten by next execution of yarn upgrade without resolutions.

@AlexTugarev
Copy link
Contributor

With yarn resolutions, it should not, right?

In case the "resolutions" fix suggested by @akosyakov does not work.

not sure if it's known already, there is a typo at https://github.com/eclipse-theia/theia/blob/master/package.json#L9, it should be resolutions

Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

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

This won't fix dependents. We also should not manually edit this lock file.

@paul-marechal
Copy link
Member

paul-marechal commented Nov 22, 2019

@akosyakov The resolutions field will update the lock file, because it effectively updates the dependency tree resolved by yarn, so it will write it down.

A better solution than the current change would be to add this to the resolutions field in this repo indeed, and advise dependents to do the same in their repositories.

An even better long term solution would be to find and fix whatever package is pulling and using the wrong version of this vscode package, since this is quite broken.

…erver

Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
@tolusha
Copy link
Contributor Author

tolusha commented Nov 25, 2019

@marechal-p @akosyakov
The PR is updated to use resolutions

@tolusha
Copy link
Contributor Author

tolusha commented Nov 25, 2019

That is pretty important for Eclipse Che, could some one validate the PR

@paul-marechal paul-marechal self-requested a review November 25, 2019 14:40
@paul-marechal
Copy link
Member

Keep in mind that this won't fix Che, unless your build system is different from just consuming Theia packages from some registry. Dependents should add the same thing you did in their own application's package.json.

@tolusha
Copy link
Contributor Author

tolusha commented Nov 25, 2019

@marechal-p
thank you!

@tolusha tolusha merged commit c79a858 into master Nov 25, 2019
@tolusha tolusha deleted the ab/fixjson branch November 25, 2019 14:54
vince-fugnitto added a commit to theia-ide/theia-apps that referenced this pull request Nov 25, 2019
Fixes #271

- fixes issue where JSON language server did not provide content assist
- fixes issue where JSON language server did not provide hover information
- fixes issue where JSON language server did not generate markers
- similar fix provided by the main repository eclipse-theia/theia#6607
- bonus: added `.editorconfig` file to have consistent formatting across the repo

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
vince-fugnitto added a commit to theia-ide/theia-apps that referenced this pull request Nov 25, 2019
Fixes #271

- fixes issue where JSON language server did not provide content assist
- fixes issue where JSON language server did not provide hover information
- fixes issue where JSON language server did not generate markers
- similar fix provided by the main repository eclipse-theia/theia#6607
- bonus: added `.editorconfig` file to have consistent formatting across the repo

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
vince-fugnitto added a commit to theia-ide/theia-apps that referenced this pull request Nov 25, 2019
Fixes #271

- fixes issue where JSON language server did not provide content assist
- fixes issue where JSON language server did not provide hover information
- fixes issue where JSON language server did not generate markers
- similar fix provided by the main repository eclipse-theia/theia#6607
- bonus: added `.editorconfig` file to have consistent formatting across the repo

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
vince-fugnitto added a commit to theia-ide/theia-apps that referenced this pull request Nov 26, 2019
Fixes #271
Fixes #233

- fixes issue where JSON language server did not provide content assist
- fixes issue where JSON language server did not provide hover information
- fixes issue where JSON language server did not generate markers
- similar fix provided by the main repository eclipse-theia/theia#6607
- bonus: added `.editorconfig` file to have consistent formatting across the repo
- bonus: added `settings.json` for development through Theia
- bonus: added `setting.json` and `extensions.json` for development through VS Code

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
vince-fugnitto added a commit to theia-ide/theia-apps that referenced this pull request Nov 26, 2019
Fixes #271
Fixes #233

- fixes issue where JSON language server did not provide content assist
- fixes issue where JSON language server did not provide hover information
- fixes issue where JSON language server did not generate markers
- similar fix provided by the main repository eclipse-theia/theia#6607
- bonus: added `.editorconfig` file to have consistent formatting across the repo
- bonus: added `settings.json` for development through Theia
- bonus: added `setting.json` and `extensions.json` for development through VS Code

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
vince-fugnitto added a commit to theia-ide/theia-apps that referenced this pull request Nov 26, 2019
Fixes #271
Fixes #233

- fixes issue where JSON language server did not provide content assist
- fixes issue where JSON language server did not provide hover information
- fixes issue where JSON language server did not generate markers
- similar fix provided by the main repository eclipse-theia/theia#6607
- bonus: added `.editorconfig` file to have consistent formatting across the repo
- bonus: added `settings.json` for development through Theia
- bonus: added `setting.json` and `extensions.json` for development through VS Code

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
dwjbosman pushed a commit to theia-ide/theia-apps that referenced this pull request Nov 28, 2019
Fixes #271
Fixes #233

- fixes issue where JSON language server did not provide content assist
- fixes issue where JSON language server did not provide hover information
- fixes issue where JSON language server did not generate markers
- similar fix provided by the main repository eclipse-theia/theia#6607
- bonus: added `.editorconfig` file to have consistent formatting across the repo
- bonus: added `settings.json` for development through Theia
- bonus: added `setting.json` and `extensions.json` for development through VS Code

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
@RomanNikitenko RomanNikitenko mentioned this pull request Jun 15, 2020
47 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants