-
Notifications
You must be signed in to change notification settings - Fork 106
Feat: nbproxy and vscode #597
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR, main comments here:
- Lot of duplications of script and content, this should not be necessary
- Main duplicated content should be if possible implemented properly as a Jupyterlab extension in another repository. The code linked to VSCode should not be implemented here.
| tmux | ||
| tree | ||
| unzip | ||
| vim No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are all this dependencies really needed for this particular change? I feel some are just here for adding tools, but could be in another PR?
| setuptools.setup( | ||
| name="jupyter-proxy", | ||
| version="0.0.1", | ||
| url="https://github.com/jupyterhub/jupyter-server-proxy/tree/master/contrib/theia", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This installation process looks complicated, especially in this repo. Couldn't we build a better extension system, maybe in the repo linked above, so that such an extension could be easily installed with standard Jupyter toolset?
| tig | ||
| tmux | ||
| tree | ||
| unzip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this normal to have to repeat all the apt package again in ml-notebook?
| tree | ||
| unzip | ||
| vim | ||
| nano |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why removing this?
Add nbproxy and vscode to base-notebook, pangeo-notebook, pytorch-notebook and ml-notebook.
Needs #600 to be merged before merging this one.