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

Remaining kernel webview layer protection #10458

Merged
merged 2 commits into from
Jun 14, 2022

Conversation

rebornix
Copy link
Member

Re #10152

It includes the remaining work for removing webview dependencies from kernels:

  • Move Jupyter Export/Import into webview/extension-side for now, as this component depends on interactive-window and notebooks
  • Move Kernel server preload into webview/extension-side for now as it depends on interactive-window, notebooks and kernel.
    • It feels like the kernel server preload should be part of kernels but currently it has assumptions of how a kernel is loaded (in a notebook or iw). We should take a look later if we should move off notebook/iw for this feature.
  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
  • Title summarizes what is changing.
  • Has a news entry file (remember to thank yourself!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • Has telemetry for feature-requests.
  • Unit tests & system/integration tests are added/updated.
  • Test plan is updated as appropriate.
  • package-lock.json has been regenerated by running npm install (if dependencies have changed).

@rebornix rebornix self-assigned this Jun 14, 2022
@rebornix rebornix marked this pull request as ready for review June 14, 2022 22:10
@rebornix rebornix requested a review from a team as a code owner June 14, 2022 22:10
@@ -27,4 +32,6 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole
IJupyterVariableDataProviderFactory,
JupyterVariableDataProviderFactory
);
serviceManager.add<INotebookExporter>(INotebookExporter, JupyterExporter);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make more sense to put this into the interactive src tree? It has nothing to do with webviews but at least it depends upon the IW code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite a few modules in webview/extension-side actually are not webview related, I guess they are under these folders as the extension used to be separated to extension-side and webview-side. I'm wondering if we can introduce a top folder for these modules/features, not sure what name to use yet, but something like contrib, plugins, integration, etc.

@rebornix
Copy link
Member Author

Would it make more sense to put this into the interactive src tree? It has nothing to do with webviews but at least it depends upon the IW code.

I'll put this in my todo list for the second round of refactoring.

@rebornix rebornix merged commit 1e35a09 into main Jun 14, 2022
@rebornix rebornix deleted the rebornix/kernel-webview-remaining branch June 14, 2022 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants