Skip to content
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

Remove the notebookContentProvider API proposal #147248

Closed
2 tasks done
Tracked by #149861
mjbvz opened this issue Apr 11, 2022 · 6 comments · Fixed by #165195
Closed
2 tasks done
Tracked by #149861

Remove the notebookContentProvider API proposal #147248

mjbvz opened this issue Apr 11, 2022 · 6 comments · Fixed by #165195
Assignees
Labels
api debt Code quality issues insiders-released Patch has been released in VS Code Insiders notebook-api upstream Issue identified as 'upstream' component related (exists outside of VS Code)
Milestone

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Apr 11, 2022

The notebookContentProvider API proposal is currently not on track for finalization. This api was introduced when we were still figuring out the model for working with notebooks and was inspired by the custom editor api. However we ultimately went with the much simpler NotebookSerializer API

Parts of the notebookContentProvider API are not well tested or supported today and maintaining it adds complexity to VS Code. We'd like to remove it, however the API is currently used by (or at least enabled for):

Please let us know if/how you are using the notebookContentProvider api and if there are any factors that would block you from migrating off of it

@alyssajotice
Copy link

@mjbvz we should have what we need to migrate here: #125757. I expect to have this done by the end of the month. I'll update this thread when we've stopped using it.

@alyssajotice
Copy link

@mjbvz here is an update: #125757 (comment)

@mjbvz mjbvz added the upstream Issue identified as 'upstream' component related (exists outside of VS Code) label May 10, 2022
@mjbvz mjbvz changed the title Figure out next steps for the notebookContentProvider API proposal Remove the notebookContentProvider API proposal May 23, 2022
@mjbvz
Copy link
Collaborator Author

mjbvz commented May 23, 2022

@alyssajotice How's Liveshare's migration been going? Run into any blockers or have any questions?

@alyssajotice
Copy link

Hey @mjbvz, I am currently working on the design docs for this. This will be a large, three-part change to the Live Share notebooks architecture, and our goal is to have this done by the fall. I'll send you the design doc when it is complete so you can stay up to date.

mjbvz added a commit to mjbvz/vscode that referenced this issue May 26, 2022
For microsoft#147248

Marks the top level types in this file as deprecated. Also hooks up `registerNotebookContentProvider` to report deprecated API usage
mjbvz added a commit that referenced this issue May 26, 2022
For #147248

Marks the top level types in this file as deprecated. Also hooks up `registerNotebookContentProvider` to report deprecated API usage
@mjbvz mjbvz modified the milestones: On Deck, September 2022 Sep 9, 2022
@mjbvz
Copy link
Collaborator Author

mjbvz commented Sep 9, 2022

Looks like liveshare is not using save, saveAs, or backup. Let's remove these from the proposal entirely

mjbvz added a commit to mjbvz/vscode that referenced this issue Sep 9, 2022
For microsoft#147248

When stepping through the liveshare code, we figured out they do not appear to be using `save`, `saveAs`, or `backup` (they return empty results for these)

This PR removes this part of the proposal so we can track just the parts of the api that they are using
mjbvz added a commit that referenced this issue Sep 12, 2022
For #147248

When stepping through the liveshare code, we figured out they do not appear to be using `save`, `saveAs`, or `backup` (they return empty results for these)

This PR removes this part of the proposal so we can track just the parts of the api that they are using
@mjbvz mjbvz modified the milestones: September 2022, October 2022 Sep 21, 2022
@mjbvz
Copy link
Collaborator Author

mjbvz commented Oct 24, 2022

All external consumers have now migrated off this api

We just need to move our tests off of this api and then we can remove it entirely: #160580

mjbvz added a commit to mjbvz/vscode that referenced this issue Nov 1, 2022
mjbvz added a commit that referenced this issue Nov 2, 2022
@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Nov 2, 2022
@mjbvz mjbvz added the debt Code quality issues label Nov 28, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Dec 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api debt Code quality issues insiders-released Patch has been released in VS Code Insiders notebook-api upstream Issue identified as 'upstream' component related (exists outside of VS Code)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants
@rebornix @mjbvz @rchiodo @alyssajotice @VSCodeTriageBot and others