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

Add jupyter-lsp to dependencies list #282

Closed
wants to merge 1 commit into from

Conversation

trungleduc
Copy link
Member

This is a part of the ongoing effort (jupyterlab/frontends-team-compass#148, jupyter-server/team-compass#30) to support LSP in core JupyterLab.

@welcome
Copy link

welcome bot commented Jun 9, 2022

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@codecov-commenter
Copy link

codecov-commenter commented Jun 9, 2022

Codecov Report

Merging #282 (0fd433e) into main (9629592) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #282   +/-   ##
=======================================
  Coverage   65.20%   65.20%           
=======================================
  Files          22       22           
  Lines        1739     1739           
  Branches      296      296           
=======================================
  Hits         1134     1134           
  Misses        521      521           
  Partials       84       84           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9629592...0fd433e. Read the comment docs.

@fcollonval
Copy link
Member

@trungleduc is this required? As far as I can tell it is only required in jupyterlab package with jupyterlab/jupyterlab#12534, isn't it?

@trungleduc
Copy link
Member Author

Yes, to make the CI pass I added https://github.com/jupyterlab/jupyterlab/blob/2fc5419ee55f71cd1771b00dc6bd51d85b72b82c/setup.cfg#L41.
We can remove it if this PR can get in.

@fcollonval
Copy link
Member

I would rather keep it as direct dependency of jupyterlab package than this one to be consistent. Otherwise this adds an unused dependency that will bring maintenance troubles - for instance in the future if jupyter-lsp API changes, you will need to update jupyterlab and jupyterlab-server (the latter just to bump the version constraint).

@fcollonval
Copy link
Member

So I'm closing this one.

@fcollonval fcollonval closed this Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants