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

Add run notebook in dedicated extension host command #9492

Merged
merged 13 commits into from
Mar 29, 2022

Conversation

rebornix
Copy link
Member

Fixes microsoft/vscode#140374

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

@rebornix rebornix requested a review from a team as a code owner March 25, 2022 17:57
@rebornix rebornix self-assigned this Mar 25, 2022
@rebornix rebornix added this to the March 2022 milestone Mar 25, 2022
@@ -88,6 +89,10 @@ export function registerTypes(serviceManager: IServiceManager) {
IExtensionSingleActivationService,
ReloadVSCodeCommandHandler
);
serviceManager.addSingleton<IExtensionSingleActivationService>(
Copy link
Contributor

Choose a reason for hiding this comment

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

You must have older bits. This folder should exist anymore.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh I see, let me rebase

Copy link
Contributor

@rchiodo rchiodo left a comment

Choose a reason for hiding this comment

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

Package-lock.json shouldn't be updated. Ian's going to do that after the release, but it looks like your running of npm install updated it.

@IanMatthewHuff
Copy link
Member

Package-lock.json shouldn't be updated. Ian's going to do that after the release, but it looks like your running of npm install updated it.

My update from main is in now. I think that at this point we want to go ahead and update package-lock.json. Now that release is branched off it's safe to do so. It would just need to merge due to my package-lock changes.

@codecov-commenter
Copy link

codecov-commenter commented Mar 25, 2022

Codecov Report

Merging #9492 (0e62643) into main (1455d0b) will increase coverage by 0%.
The diff coverage is 48%.

❗ Current head 0e62643 differs from pull request most recent head 84d21f5. Consider uploading reports for the commit 84d21f5 to get more accurate results

@@          Coverage Diff           @@
##           main   #9492     +/-   ##
======================================
  Coverage    73%     73%             
======================================
  Files       191     229     +38     
  Lines      8854   10524   +1670     
  Branches   1303    1541    +238     
======================================
+ Hits       6482    7746   +1264     
- Misses     1857    2149    +292     
- Partials    515     629    +114     
Impacted Files Coverage Δ
...pplication/commands/runInDedicatedExtensionHost.ts 44% <44%> (ø)
src/platform/common/serviceRegistry.ts 100% <100%> (ø)
src/platform/logging/logger.ts 52% <0%> (-18%) ⬇️
src/platform/logging/transports.ts 78% <0%> (-5%) ⬇️
src/platform/common/constants.ts 96% <0%> (-2%) ⬇️
src/platform/api.ts 57% <0%> (-2%) ⬇️
src/platform/errors/types.ts 77% <0%> (-1%) ⬇️
src/platform/errors/errorHandler.ts 64% <0%> (-1%) ⬇️
src/platform/common/types.ts 100% <0%> (ø)
src/platform/messageTypes.ts 100% <0%> (ø)
... and 111 more

import { ICommandManager, IWorkspaceService } from '../types';

/**
* Prompts user to reload VS Code with a custom message, and reloads if necessary.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it actually prompt? I don't see that happening in the code

Copy link
Contributor

@DonJayamanne DonJayamanne left a comment

Choose a reason for hiding this comment

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

not sure why the package-lock is changing.
we have a debt item to address this (@rchiodo @IanMatthewHuff Ajay aware of this issue) please could you revert the changes to the package lock file

@IanMatthewHuff
Copy link
Member

Updated the package lock. Something is off with linting still. Looking into it quick.

@IanMatthewHuff
Copy link
Member

Small tslint fix on the affinity max check.

@IanMatthewHuff IanMatthewHuff merged commit 677c761 into main Mar 29, 2022
@IanMatthewHuff IanMatthewHuff deleted the rebornix/updateaffinity branch March 29, 2022 18:08
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.

Explore running notebook extension in dedicated extension host
6 participants