-
Notifications
You must be signed in to change notification settings - Fork 160
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
Migration to jupyterlab 4.0 #659
Comments
Hi,
|
Hey @vidartf I'm supporting @HaudinFlorence in that migration effort. Could you elaborate the reasons for supporting 3 + 4? I don't see them as basically this project does not get much updates and it adds a huge amount of troubles to get the backward compatibility. I would rather have us spending time migrating nbdime to JupyterLab 4 / Lumino 2 / CodeMirror 6 rather than fighting with the backward compatibility that will likely be Lab 3 | 4 / Lumino 2 | 1 / CodeMirror 5. |
@fcollonval To try to rephrase myself: I want to add support for JupyterLab 4. The amount of work needed to do that depends on how many things we use break with the lumino 2.0 upgrade. If there are no breakages, we might even be able to support both 3.0 and 4.0. Separately there is the question of the Codemirror change in JupyterLab 4.0. As of yet, the CM "DiffView/MergeView" code that I used as a basis for https://github.com/jupyter/nbdime/blob/master/packages/nbdime/src/common/mergeview.ts has not been ported to CM6, so I do not think that we can quickly change to using CM6 for nbdime. Hopefully, it should not be a problem for us to continue using CM5 in nbdime with JupyterLab 4.0 though. I'd be interested to hear your thoughts on whether both of them can be on the page at the same time. |
Or maybe the mergeview code has been ported now? https://github.com/codemirror/merge not sure if this is it? |
I think it should work as the API are totally unrelated (of course there is a performance cost but at least that would ease a first migration step). When trying to load nbdime without change in JupyterLab-git for JLab 4, the major issue seen is related to webpack configuration as for JLab 3, nbdime expect codemirror to be shared. Investigation is under way to see how to pass/fix that.
We saw that repo too. But we have not checked it yet to see how it compares with the addon of CM5. |
It looks like the MergeView for CM6 supports the two-way view only, but it should be possible (and not too much complicated) to build a 3-way mergeview on top of it. |
Could you also add dependency support for jupyter_server 2.X.X please. Currently it only supports 1.X.X and lots of other extensions use 2.X.X. Thanks! :) |
Note: An alpha version was released with #673 |
I started a task list in the first comment of this issue to track remaining issues to be addressed. |
cc @krassowski - see tasks list; I proposed that contributor set them self as assignee when they start working on an issue to avoid conflict. |
@vidartf @krassowski there are 4 nice to have PRs opened:
Mike also mentioned #639 Do we have other things mandatory for having a new final release of nbdime? |
Not mandatory, but it would be nice to have #636 as well, but that needs rebasing (pick up autoformat changes, cm6 changes) + needs some theming attention (new colors are not transparent, so struggle on themes, icons need to be reviewed if styleable by lab theme). Just mentioning for completeness. |
Also: #720 |
Possibly #700 should not block the release, especially because after investigation it does not appear to be a regression due to CM6 migration? |
@vidartf @fcollonval It looks like we have everything that we wanted in for this release. Should we cut a beta or move straight to RC phase and announce intent to make a final release next week if nothing major comes up? |
I'm 💯 on moving to rc and cut a final release as soon as possible. |
I tried to publish an RC using the releaser but it failed (https://github.com/jupyter/nbdime/actions/runs/6770648492) because of insufficient rights. Can someone else can pick it up or help with permissions? |
Thanks Mike I updated your rights. And I'm gonna give it a try. I'm not sure the version of the npm packages are gonna be set up properly with the releaser 🤞 |
... the JS packages are kept as alphas ... I'm gonna do a PR to migrate to rc.0. So they will be published as |
4.0.0rc0 has been released on PyPI The current limitations of the
|
Does the bump-version script ask for bumps independently for each target? Can we remove the private npm packages (webapp, root) as targets? It doesn't really do anything, and adds git noise. Or is that what you are saying is happening? |
FYI: I see the tag format changed from 3.2.1 --> v4.0.0rc1. Is that something we can normalize? |
The script does not ask anything as it is run within GitHub jobs. It tries to be smart. But we may have to do something like lumino; forcing us to create a PR to bump the NPM package versions before releasing.
We can set RH_TAG_FORMAT in that environment, I think. |
Should we try to also get #718 in before release? The notebook metadata editors all collapsed bit. |
Could we define an ETA for the final release? |
I think we can release today or tomorrow, and then do any of the remaining nice to haves in a quick minor release after? |
Not sure if that is possible, since it seems to be hardcoded other places as well: |
Do we want to wait for jupyter-server/jupyter_releaser#536? |
If it's a short wait that would be nice. Not sure how easy/hard it would be to clean up manually? |
I'm doing a last rc right now to test the new tag format. |
Here it goes: https://github.com/jupyter/nbdime/releases/tag/4.0.0rc1 The tag is correct - I needed to pin a custom fork of the releaser to use the bug fix. So I opened #738 as follow-up. I'll do the final release this afternoon (around 4pm CET); let me know here if you want to postpone it. |
🥁 i'm starting the final release |
Ok final version has been released: https://github.com/jupyter/nbdime/releases/tag/4.0.0 |
But I think it is broken 💥 as CI is failing ---> investigating |
Ok the error is due to the parsing of the version without release level. I yanked 4.0.0 on PyPI. |
4.0.1 is all green |
Thanks @fcollonval and congrats on first release of nbdime! 🎉 |
Hello,
@vidartf I am about to work on the migration of jupyterlab-git extension to jupyterlab 4.0 that depends on nbdime. Could you please give a hint about what will be the strategy concerning nbdime: is it planed to keep the compatibility with previous versions of jupyterlab or not?
Many thanks in advance.
cc @fcollonval
Tasks
The text was updated successfully, but these errors were encountered: