Skip to content

Update widget output params #5203

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

Merged
merged 3 commits into from
Mar 18, 2021
Merged

Conversation

IanMatthewHuff
Copy link
Member

For #5195

  • 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 enhancements.
  • 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).

if (customMetadata.unconfined) {
metadata.unconfined = customMetadata.unconfined;
}
if ('mime' in request) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Function looked the same as the function that I updated in renderers extension so same update here as well.

@IanMatthewHuff
Copy link
Member Author

@DonJayamanne Is there a way to test this in stable as well? Can't just build and debug in stable as the stable engine isn't 1.55. With the renderers I copied over the client_renderer to the jupyter extension installed in stable to test. Can I just dump my out dir into the stable extension folder?

I did check manualTestFiles (which has rich outputs + a few widgets) on insiders both with the old renderer code and my new updated renderer code and it looked legit to me.

@IanMatthewHuff IanMatthewHuff marked this pull request as ready for review March 18, 2021 18:10
@IanMatthewHuff IanMatthewHuff requested a review from a team as a code owner March 18, 2021 18:10
@IanMatthewHuff IanMatthewHuff changed the title Dev/ianhu/update widget output params Update widget output params Mar 18, 2021
@DonJayamanne
Copy link
Contributor

Can't just build and debug in stable as the stable engine isn't 1.55. With the renderers I copied over the client_renderer to the jupyter extension installed in stable to test. Can I just dump my out dir into the stable extension folder?

When we create a new release, we'll pin against the next version of VS Code (which is the current insider).
Hence no need to test in Stable at all.

data: { [mimeType: string]: any };
metadata?: OldNotebookCellOutputMetadata;
}
interface OldNotebookOutputEventParams {
Copy link
Contributor

Choose a reason for hiding this comment

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

@IanMatthewHuff I think we can remove this code and there's no need for backwards compatiblity.
The only reason i wanted the backwards compatiblity for renderers, is so that it works with point releases of jupyter extension (point release is for current stable).

Copy link
Member Author

Choose a reason for hiding this comment

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

@DonJayamanne Ahh, that's a good call. Wasn't thinking when I moved that code over. You are right, this will only release with matching stable builds. I'll remove that quick.

@IanMatthewHuff IanMatthewHuff merged commit 2fb6da2 into main Mar 18, 2021
@IanMatthewHuff IanMatthewHuff deleted the dev/ianhu/updateWidgetOutputParams branch March 18, 2021 20:34
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.

3 participants