-
Notifications
You must be signed in to change notification settings - Fork 369
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
Remove most configuration, get this working #115
Conversation
Thanks for submitting your first pull request! You are awesome! 🤗 |
Been struggling with getting this repo running yesterday and today. Glad I found this PR as works instantly :) Makes sense to me that this should be reviewed/merged by the admin |
My only thing would be the PRs should be smaller to more easily follow and merge. There are lots of things introduced in one go. Smaller PRS are easier as incremental improvements. |
It is a big PR, but this repo is pretty stale, so I think a big restart to get things back in shape is fine at the moment. |
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.
Go ahead and merge if you're happy with it @manics
It's a big change.... but it works so I think this is a good place to build upon. |
This repo is currently broken https://discourse.jupyter.org/t/jupyterhub-deploy-docker-maintainer-update/14919
I've rewritten and simplified the repo so it should now work and be easier to maintain. However it might be better to archive this repository, and make this an example under https://github.com/jupyterhub/dockerspawner/tree/main/examples (which are also out of date- we should add some tests there).
What do you think?