-
Notifications
You must be signed in to change notification settings - Fork 294
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
multiplexingDebugService for the web #10070
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.
I think you could instead rename multiplexingDebugService.node.ts
to multiplexingDebugService.ts
and just have the node specific parts optional.
@rchiodo can do! thank you for the feedback 👍 |
Codecov Report
@@ Coverage Diff @@
## main #10070 +/- ##
======================================
- Coverage 64% 64% -1%
======================================
Files 216 215 -1
Lines 10060 10047 -13
Branches 1613 1619 +6
======================================
- Hits 6508 6456 -52
- Misses 3028 3064 +36
- Partials 524 527 +3
|
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.
Looks good, just some minor fixups.
@IanMatthewHuff @rchiodo thank you so much for your kind and thoughtful reviews! I will polish this up on Monday. 🙏 thanks again! |
Migrated the multiplexingDebugService to the web, for #9665
This PR gets us closer to the meatier section of the migration of the variable views. The highlight in this case is making the
IJupyterDebugService
optional in themultiplexingDebugService
.Feedback appreciated!