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.

Sign up for GitHub

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

Jump to bottom

core: fix import in lsp-types.ts #7075

Merged
merged 1 commit into from
Feb 4, 2020
Merged

core: fix import in lsp-types.ts #7075

merged 1 commit into from
Feb 4, 2020

Conversation

paul-marechal
Copy link
Member

Signed-off-by: Paul Maréchal [email protected]

What it does

Fixes an import statement in core to refer to a package that is part of its dependencies.

See #7056 (comment)

How to test

This should allow applications relying on core only to correctly build. In the meantime, we can check that vscode-languageserver-types is an actual dep of @theia/core.

Review checklist

Reminder for reviewers

@paul-marechal
core: fix import in lsp-types.ts
82c8778 
Import statement is refering to a package which is not part of core's
dependencies.

Signed-off-by: Paul Maréchal <[email protected]>
@paul-marechal paul-marechal added bug bugs found in the application core issues related to the core of the application labels Feb 4, 2020
@akosyakov
Copy link
Member

btw i thought we have lint rule which disallows using stuff which is not listed in dependencies. how that get through?

akosyakov
akosyakov approved these changes Feb 4, 2020
View reviewed changes
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

lgtm

@paul-marechal paul-marechal merged commit 8cf1dec into master Feb 4, 2020
@paul-marechal paul-marechal deleted the mp/lsp-types branch February 4, 2020 18:38
@vince-fugnitto
Copy link
Member

vince-fugnitto commented Feb 4, 2020 •
edited
Loading

btw i thought we have lint rule which disallows using stuff which is not listed in dependencies. how that get through?

@akosyakov eslint does not support no-implicit-dependencies :( we will need to find a way (through an extra plugin or custom rule) to get the same behavior.

eslint/eslint#11501

@paul-marechal
Copy link
Member Author

The bogus import predates that, but it is true that we need to bring the rule back with eslint.

@paul-marechal
Copy link
Member Author

@akosyakov the change went through because vscode-languageserver-protocol was whitelisted in the tslint config for the no implicit dep rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akosyakov akosyakov akosyakov approved these changes

Assignees
No one assigned
Labels
bug bugs found in the application core issues related to the core of the application
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@paul-marechal @akosyakov @vince-fugnitto