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 schema descriptions #11

Closed
wants to merge 4 commits into from
Closed

Conversation

choldgraf
Copy link

This updates a few of the schema descriptions so that they are more clear.

frontend/src/schema.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@GeorgianaElena GeorgianaElena left a comment

Choose a reason for hiding this comment

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

🚀

@yuvipanda
Copy link
Owner

This also needs the frontend js to be rebuilt I think before it can be deployed... Need to automate that.

Copy link

@damianavila damianavila left a comment

Choose a reason for hiding this comment

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

As per @yuvipanda's last comment, this one needs the JS to be rebuilt.

From @GeorgianaElena

I generated this running npm run dev

@choldgraf
Copy link
Author

OK I think I just did this, let me know if I got it wrong.

While I was at it, I updated the contributing docs a bit, because they were a little unclear to me :-)

@manics
Copy link

manics commented Mar 11, 2022

Need to automate that.

I had a go at copying jupyterhub/nbgitpuller#211 + jupyterhub/nbgitpuller#222 to this repo (untested, and has some bugs):
https://github.com/yuvipanda/jupyterhub-configurator/compare/main...manics:package?expand=1

It's a bit more complicated than nbgitpuller because of the extra webpack/babel/css config and subdirectory.

I was also puzzled by index.html:

<head><script defer src="index.js"></script></head>Html Webpack Plugin:
<pre>
Error: Child compilation failed:
Module not found: Error: Can't resolve '/Users/yuvipanda/code/jupyterhub-confi gurator/frontend/src/index.html' in '/Users/yuvipanda/code/jupyterhub-configur ator/frontend'
ModuleNotFoundError: Module not found: Error: Can't resolve '/Users/yuvipanda/ code/jupyterhub-configurator/frontend/src/index.html' in '/Users/yuvipanda/cod e/jupyterhub-configurator/frontend'
- Compilation.js:1668
[frontend]/[webpack]/lib/Compilation.js:1668:28
- NormalModuleFactory.js:712
[frontend]/[webpack]/lib/NormalModuleFactory.js:712:13
- NormalModuleFactory.js:273
[frontend]/[webpack]/lib/NormalModuleFactory.js:273:22

@choldgraf
Copy link
Author

@manics I found it confusing too haha, that's why I tried to document a little bit more.

This feels like something that nox or tox could be useful for. Then a person could just run something like nox -s compile and it would (1) install npm in a virtual environment if needed, (2) run the proper npm commands to build the assets needed. We use this in the Jupyter Book world a lot and it helps reduce confusion associated with installing NPM environments

@manics
Copy link

manics commented Mar 11, 2022

The idea behind Jupyter-packaging (or the equivalent code) is to include the npm build commands in setup.py so the usual pip install . takes care of almost everything, and you never need to commit the generated JS assets- they're generated and packaged automatically into the wheel.

@yuvipanda
Copy link
Owner

@manics wow that would be awesome. Can you make a PR too?

@manics
Copy link

manics commented Mar 11, 2022

Will do when I get time to test it (assuming it works!)

@choldgraf
Copy link
Author

@manics you should check out the Sphinx Theme Builder that @pradyunsg built, it uses the pyproject.toml build standard to include an npm environment with an install. It is lovely: https://github.com/pradyunsg/sphinx-theme-builder

@manics
Copy link

manics commented Mar 12, 2022

pyproject.toml/pep517 is nice, but if you're using setuptools you'll loose the option of an editable install (pip install -e .) 😢 pypa/setuptools#2816

I've therefore gone for a build without pep517 for now in #13
If you merge your current PR I can update the docs in my PR

@choldgraf
Copy link
Author

@manics yeah, we have switched to using flit for builds in jupyter book, which gives you editable builds 🙂

For this PR, I think it's good to go? I don't know that I can merge into @yuvipanda 's repo though

@pradyunsg
Copy link

pyproject.toml/pep517 is nice, but if you're using setuptools you'll loose the option of an editable install (pip install -e .) 😢 pypa/setuptools#2816

Yes, but if you're doing something like this in a custom build backend, you won't be relying on setuptools to perform the build for you. Basically, instead of:

The idea behind Jupyter-packaging (or the equivalent code) is to include the npm build commands in setup.py so the usual pip install .

You'd end up doing for sphinx-theme-builder is doing -- running npm commands before packaging things up yourself.

@choldgraf
Copy link
Author

Can somebody tell me if there's anything else I should do, or if we can merge this?

@yuvipanda
Copy link
Owner

Unfortunately this project is an architectural dead end - see 2i2c-org/features#26. Going to close this one out.

@yuvipanda yuvipanda closed this Mar 21, 2024
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

Successfully merging this pull request may close these issues.

6 participants