Migrate to Jupyter Lab v4 #135
Conversation
js/package-lock.json
Outdated
There was a problem hiding this comment.
Given that you are switching from npm to yarn we may want to use yarn in the package.json scripts
There was a problem hiding this comment.
Hi,
thank you for the work!
I would prefer however if all unrelated changes are not pushed to this branch.
For ipyreact we added styling like this: widgetti/ipyreact#51
using ruff + prettier but that should not go into this PR.
Also, does a build with jupyterlab4 make ipywebrtc still work in jupyterlab3.x?
I'm happy to have formatting/style fixes btw, but not mixed with other PR's :)
Hope I don't discouraged you too much, feel free to push back.
Cheers,
Maarten
.github/workflows/main.yml
Outdated
| - name: Checkout | ||
| uses: actions/checkout@v2 | ||
|
|
||
| - name: Install Conda environment with Micromamba | ||
| uses: mamba-org/setup-micromamba@v1 | ||
| with: | ||
| environment-file: dev_environment.yml | ||
| condarc: | | ||
| dependencies: | ||
| - python==${{ matrix.python-version }} | ||
|
|
||
| - name: Test flake8 | ||
| run: flake8 ipywebrtc --ignore=E501,F405 | ||
|
|
||
| - name: Install ipywebrtc | ||
| run: pip install . | ||
|
|
||
| - name: Check installation files | ||
| run: | | ||
| test -d $CONDA_PREFIX/share/jupyter/nbextensions/jupyter-webrtc | ||
| test -f $CONDA_PREFIX/share/jupyter/nbextensions/jupyter-webrtc/extension.js | ||
| test -f $CONDA_PREFIX/share/jupyter/nbextensions/jupyter-webrtc/index.js | ||
| test -d $CONDA_PREFIX/share/jupyter/labextensions/jupyter-webrtc | ||
| test -f $CONDA_PREFIX/share/jupyter/labextensions/jupyter-webrtc/package.json | ||
|
|
||
| - name: Check nbextension and labextension | ||
| run: | | ||
| jupyter labextension list 2>&1 | grep -ie "jupyter-webrtc.*enabled.*ok" - | ||
|
|
||
| - name: Run js tests | ||
| run: | | ||
| npm install | ||
| npm run test | ||
| working-directory: js | ||
|
|
||
| - name: Build docs (Only on MacOS for build speed) | ||
| if: matrix.os == 'macos-latest' | ||
| run: | | ||
| cd docs/source/ | ||
| sphinx-build . _build/html | ||
| cd ../.. |
There was a problem hiding this comment.
Please avoid unneeded changes/reformatting, or put them in a separate PR or top commit first. This makes tracking changes much easier.
There was a problem hiding this comment.
I understand what you're saying, I can do a new PR for style changes.
| - name: Checkout | ||
| uses: actions/checkout@v2 | ||
|
|
||
| - name: Checkout | ||
| uses: actions/checkout@v2 | ||
|
|
||
| - name: Install Conda environment with Micromamba | ||
| uses: mamba-org/provision-with-micromamba@main | ||
| with: | ||
| environment-name: ipywebrtc-dev | ||
| environment-file: dev_environment.yml | ||
| python-version: ${{ matrix.python-version }} | ||
| mamba-version: "*" | ||
| auto-activate-base: false | ||
| channels: conda-forge | ||
|
|
||
| - name: Build packages | ||
| run: | | ||
| python setup.py sdist bdist_wheel | ||
| cd dist | ||
| sha256sum * | tee SHA256SUMS | ||
| - name: Upload builds | ||
| uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: dist ${{ github.run_number }} | ||
| path: ./dist | ||
| - name: Install Conda environment with Micromamba | ||
| uses: mamba-org/setup-micromamba@v1 | ||
| with: | ||
| environment-file: dev_environment.yml | ||
| condarc: | | ||
| dependencies: | ||
| - python==${{ matrix.python-version }} | ||
|
|
||
| - name: Build packages | ||
| run: | | ||
| python setup.py sdist bdist_wheel | ||
| cd dist | ||
| sha256sum * | tee SHA256SUMS | ||
| - name: Upload builds | ||
| uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: dist ${{ github.run_number }} | ||
| path: ./dist |
There was a problem hiding this comment.
Same here, very difficult to see what actually changed and what is related to the intention or the PR
There was a problem hiding this comment.
It's easier to review hiding the whitespaces changes https://github.com/maartenbreddels/ipywebrtc/pull/135/files?diff=split&w=1
There was a problem hiding this comment.
w=1 is the query parameter
| - python: "3.6" | ||
| dist: "ipywebrtc*.tar.gz" | ||
| - python: "3.9" | ||
| dist: "ipywebrtc*.whl" |
There was a problem hiding this comment.
This isn't that bad, because it's not mixed with other changes, but adds noise.
| import * as services from "@jupyterlab/services"; | ||
| import * as Backbone from "backbone"; | ||
| import * as widgets from "@jupyter-widgets/controls"; | ||
| import * as base from "@jupyter-widgets/base"; | ||
| import * as sinon from "sinon"; |
There was a problem hiding this comment.
Also the changes in this file are not needed, although I welcome a style fix :)
No description provided.