-
Notifications
You must be signed in to change notification settings - Fork 62
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 news banner and log #215
Add news banner and log #215
Conversation
@jupyter/notebook-council As this PR introduces a very visible change for the users, I am looking for any feedback/enhancement. |
Thx @RRosio for the review. Let's leave this open a few more days for any feedback before merging. |
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 giving us a chance to look at this! It looks solid to me.
nbclassic/templates/page.html
Outdated
<div style="display: flex"> | ||
<div> | ||
<span class="label label-warning">NEWS</span> | ||
If you plan to upgrade to Notebook 7, read <a href="https://nbclassic.readthedocs.io/en/latest/nbclassic_notebook.html" target="_top">https://nbclassic.readthedocs.io/en/latest/nbclassic_to_notebook7.html</a> |
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.
Both the links in this line give me 404 errors. I'm guessing this is intentional and these docs aren't being pushed to latest
until the release, but I wanted to note it just in case.
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.
Once the RDT website deployed (automatically done on PR merge), the 404 will be fixed.
Before releasing, we will double-check that the link is working fine.
Co-authored-by: Isabela Presedo-Floyd <[email protected]>
Co-authored-by: Isabela Presedo-Floyd <[email protected]>
Thx a lot @isabela-pf. I have merged your suggestions. |
Question - isn't our plan to deprecate and not support notebook 6 moving forward. Do we have a date as to when notebook 6 will stop being supported? If that is the case, don't we want to phrase this to communicate that to users? The phrasing "if you plan..." suggests that upgrading is entirely optional for users. |
The JEP on noteobook 7 says "Jupyter Notebook v6 will be maintained with critical security fixes for two years following the release of Jupyter Notebook v7." I think the banner should communicate that to users. |
I should note though that I think a banner like this is a fine idea and will be helpful for users to understand the plan. |
Yes, that is the plan
We had a few discussions in the community meeting related to support policies for Notebook, but nothing has been proposed nor decided to my knowledge and understanding.
Well, I think the upgrade is optional indeed. As we don't have a EOL support date, I guess we could say "Notebook 6... will soon be deprecated..." Please note that this banner will for now be shipped for NbClassic, so we may remove the mention to Notebook 6 to make the message more clear. Also the primary intent of this banner is to highlight that any custom Notebook extensions will stop working with Notebook 7, instead of focussing on any depreciation or invitation to migrate to Notebook 7. |
Thank you for the pointer. Do you have a content that we could use as text for the banner? |
BTW since the JEP, Notebook has evolved to NbClassic as a Jupyter Server extension, not sure if that 2 years support also applies to NbClassic. |
The result of the Notebook community meeting discussions can be read on jupyter/notebook-team-compass#5 (comment)
|
The changes are at the nbclassic template and jupyter server extensions levels. As such, they will not show up in Notebook 6.x distributions, even after a new nbclassic release. We can thus open other PR on the Notebook branches to add similar features (banner + log), but with different messages and focus if needed. |
Co-authored-by: Afshin Taylor Darian <[email protected]>
Co-authored-by: Afshin Taylor Darian <[email protected]>
Co-authored-by: Afshin Taylor Darian <[email protected]>
I have updated this PR based on feedbacks collected here and in #214, and opened https://github.com/jupyter/notebook/pull/6737/files |
@RRosio jupyter/notebook#6737 is merged and https://jupyter-notebook.readthedocs.io/en/latest/migrate_to_notebook7.html is lie This PR is good to merge so we can cut a release. |
@@ -147,6 +84,3 @@ Jupyter Notebook 7. | |||
**Porting Notebook 6 Extensions**: Work being done in parallel. | |||
ou can find a helpful list of classical Notebook extensions and corresponding Jupyterlab extensions | |||
if available at the `Jupyterlab-contrib website <https://jupyterlab-contrib.github.io/migrate_from_classical.html>`_. | |||
|
|||
|
|||
.. _JEP #79: https://jupyter.org/enhancement-proposals/79-notebook-v7/notebook-v7.html |
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.
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.
I have added back the link to the JEP 79
CI is failing with
|
I read that |
I have relaunched the CI and the intermediary 502 returned by unpkg is now fixed. @RRosio this is good to merge. Thx. |
Related to #214
This PR add a banner and log message. The user can click on a button to not see the banner anymore (that choice is persisted across session and server restart via the local storage).