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

Update dev setup documentation #411

Closed
karlaspuldaro opened this issue Nov 16, 2020 · 4 comments
Closed

Update dev setup documentation #411

karlaspuldaro opened this issue Nov 16, 2020 · 4 comments

Comments

@karlaspuldaro
Copy link
Contributor

Following the dev steps in CONTRIBUTING.md I came across a few issues:

  1. Wrong command to create a new conda environment:
$ conda env update -n jupyterlab-lsp
SpecNotFound: Invalid name, try the format: user/package

Conda version: 4.9.2
Solution: conda create -n jupyterlab-lsp

  1. Node version expected to be 12+
$ jlpm
yarn install v1.21.1
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
error @typescript-eslint/eslint-plugin@3.10.1: The engine "node" is incompatible with this module. Expected version "^10.12.0 || >=12.0.0". Got "10.0.0"
error Found incompatible module.
  1. Recommend Python 3.6+
    As per jupyterlab requirements
@krassowski
Copy link
Member

Fantastic work! Thank you. Please note that node 12 is not required. The message you see says:

Expected version "^10.12.0 || >=12.0.0".

meaning that the old versions of node 10 are not supported. We do run CI tests on node 10 (10.15.3 at current), see https://github.com/krassowski/jupyterlab-lsp/runs/1414536659?check_suite_focus=true#step:6:156

However, now that you have changed it on #412 I am willing to consider accepting the change if @bollwyvl agrees. On one hand, it would make sense to recommend the newer version, on the other the 10.x will not be EOL before 2021-04-30.

@bollwyvl
Copy link
Collaborator

bollwyvl commented Nov 17, 2020 via email

@karlaspuldaro
Copy link
Contributor Author

Thank you for the feedback, I agree with all being said.
I can change notes back to node 10 on my PR if the plan is to continue testing it until next year.
Since I had node 10.0.0 and it wasn't enough, in that case I suggest updating the docs with the minimum expected then (10.12.0 from the logs)? Then make it more clear in #405 that node12 is required for devs?

@krassowski
Copy link
Member

I think that this was fixed already. Many thanks @karlaspuldaro!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants