-
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
Add initial cell identifiers awareness #566
Conversation
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.
Thanks for picking up the slack here. I have some changes locally that complements this (test changes only). I'll do a PR to your branch with them.
All tests that were failing due to introduction of cell ids are now passing. There are still failing tests, but unrelated:
|
Comparing jupyter_server/search?q=mathjax_url and notebook/search?q=mathjax_url it looks that:
|
Ideally, we would want to check that ids are properly merged/removed, but this is ok as a stop-gap to get tests running
Thanks! 🥇 |
To keep the conversation from #553 going, in particular the step 1:
I am opening a draft PR that might help with conceptualizing what is needed to move forward. The number of failing tests is down from 34 to 32 (fixed
test_pretty_print_code_cell
andtest_git_diff_driver_add_helper_filter
I think) but the bulk of merge tests is still failing. They are now failing for a different reason (I temporarily set"/cells/*/id": "remove"
while awaiting feedback to highlight the file where the changes are likely needed).I really wish we could get nbdime out of beta to also allow jupyterlab-git to be properly released and for the lab ecosystem users to migrate to 3.0 and reduce the maintenance burden on the older versions of other Jupyter software too.
This PR might help to close #553.