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

move process utilities to jupyterlab_launcher #4696

Merged
merged 1 commit into from
Jun 13, 2018
Merged

move process utilities to jupyterlab_launcher #4696

merged 1 commit into from
Jun 13, 2018

Conversation

ivanov
Copy link
Member

@ivanov ivanov commented Jun 8, 2018

ivanov
ivanov commented Jun 8, 2018
View reviewed changes
packages/services/examples/node/main.py
from jupyterlab.process import which
from jupyterlab.process_app import ProcessApp
from jupyterlab_launcher.process import which
from jupyterlab_launcher.process_app import ProcessApp
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the kind of clear indication that the functionality of which and ProcessApp belong outside of jupyterlab, since otherwise this standalone example services package still has a dependency on all of jupyterlab.

Copy link
Contributor

Choose a reason for hiding this comment

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

Very good point.

@ivanov
Copy link
Member Author

ivanov commented Jun 8, 2018

This cannot be merged until a new version of jupyterlab_launcher is released. /cc @jasongrout

@jasongrout jasongrout added this to the Beta 3 milestone Jun 8, 2018
@jasongrout
Copy link
Contributor

This cannot be merged until a new version of jupyterlab_launcher is released. /cc @jasongrout

A release is ready to go, just waiting on permission to upload it.

@jasongrout
Copy link
Contributor

The pypi release of jupyterlab_launcher 0.11, with these changes, is uploaded, and the conda-forge release is in-process: conda-forge/jupyterlab_launcher-feedstock#33

@jasongrout
Copy link
Contributor

Restarted the tests now that the pypi release is out.

@jasongrout
Copy link
Contributor

I think we should hold off merging this until just before our next pre-release, since it forces everyone to update jupyterlab_launcher.

afshin
afshin approved these changes Jun 13, 2018
View reviewed changes
Copy link
Member

@afshin afshin left a comment

Choose a reason for hiding this comment

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

Awesome. Thank you, @ivanov!

@afshin afshin merged commit a76fd32 into jupyterlab:master Jun 13, 2018
@vidartf vidartf mentioned this pull request Jun 14, 2018
Merged
@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Aug 8, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jasongrout jasongrout jasongrout left review comments

@afshin afshin afshin approved these changes

Assignees
No one assigned
Labels
status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Milestone
0.33
Development

Successfully merging this pull request may close these issues.
3 participants
@ivanov @jasongrout @afshin