Use jupyter-packaging for npm build

[manics]

This is one possible way of solving #210

Needs some testing.

Closes #210

[consideRatio]

@manics nice!!! This looks great!

Do we need to update MANIFEST.in somehow? This is the current MANIFEST.in file. I consider pyproject.toml and/or package.json

include *.md
include LICENSE
include setup.cfg
recursive-include nbgitpuller/static *
recursive-include nbgitpuller/templates *

UPDATE: I think I've concluded that we should not include pyporject.toml or package.json, but we should use a try/catch block within setup.py as I note in a comment below.

[consideRatio]

I think you should wrap this in a try catch block, so that you don't assume jupyter_packaging to be available.

See this example from the jupyter_packaging readme file https://github.com/jupyter/jupyter-packaging#as-a-build-requirement

I'm thinking that if we have built the package so that the bundle.js stuff is available, then we are fine. If we on the other hand just want to install things from a package source distribution, we don't need pyproject.toml, package.json, or anything - we just need the bundle to be available and keep working as normal in setup.py.

[manics]

I originally followed the example, but that means setup.py can silently ignores the js build which didn't seem particularly great.

It looks like pyproject.toml is included anyway in the sdist (at least it is when I tested locally, and pip install ~/path/to/nbgitpuller/dist/nbgitpuller-1.0.1.dev0.tar.gz works without errors).

What do you think?

[manics]

Long discussion here which I don't fully understand pypa/setuptools#1650
... but it looks like this project assumes pep517 anyway

nbgitpuller/.github/workflows/publish.yml

Line 27 in 4ffd009

python -m build --sdist --wheel .

[consideRatio]

Nice investigation @manics! Okay my understanding now:

1. pyproject.toml will always ship with the source when we package the source with build --sdist --wheel . This means that jupyter_packaging can be assumed in setup.py, so we don't need to wrap the import statement etc in a try block.
2. The nbgitpuller/static/dist content will also be shipped, and jupyter_packaging will not attempt to do a npm setup unless bundle.js isn't around anyhow because of skip_if_exists=jstargets. This means that we currently can do pip install . from the source file. If you on the other hand wants to rebuild the bundle.js file you won't be able to do so because then we miss package.json. I've verified this part.

I suggest we include package.json in our MANIFEST.in file as part of this PR.

I can't make a convenient code suggestion about that so I pushed a commit. If it LGTY also @manics then go for a merge! This LGTM now!

[GeorgianaElena]

Many thanks @manics for fixing the mess I've created 😬

This looks good to me too and I think we should merge it and release a new version as soon as possible.

P.S. jupyter-packaging seems very cool and I wish I knew about it before.

[consideRatio]

LGTM - note that I pushed a commit to include package.json in the source distribution!

[manics]

Sounds fine to me!

What's everyone's thoughts on switching to setuptools-scm in a follow-up PR? Main benefits include:

• source controlled files are included without MANIFEST.in, so only generated files need to be added
• releases should be easier, versions are obtained from the git tag instead of manually bumping the version each time

[manics]

Is the version in package.json used anywhere (it doesn't match the nbgitpuller version)? If not could we set it to something that's clearly a placeholder value like 0.0.0?

nbgitpuller/package.json

Line 3 in 4ffd009

"version": "0.10.1",

[consideRatio]

@manics I don't yet understand the tool, but I react with aversion of it because of how I perceive it to increase the complexity of things given that one need to understand what it does.

Sometimes, it's fine to not need to understand how things work, but we just come out of a situation where we needed to understand the details of this.

I think unless we look to automate everything in the release process to a single button click or similar, then I'd like to accept the work of following the current RELEASE.md procedure. Regarding package.json's version, I noted that being outdated as well and submitted #212.

[consideRatio]

Thanks @manics!!!

Many thanks @manics for fixing the mess I've created

We all had to learn more about this complexity, and thanks to your efforts to move this onwards we arrived in a situation were we realized that! Thank you for your work @GeorgianaElena! Without your work, the JMTE hub's user image failed btw, so it also had a direct value to JMTE.