Merge remote kernel finders. #11879
Merge remote kernel finders. #11879
Conversation
| @@ -56,6 +56,9 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage, IServe | |||
| public get onDidChange(): Event<void> { | |||
| return this._onDidChangeUri.event; | |||
| } | |||
| public get onDidChangeConnectionType(): Event<void> { | |||
There was a problem hiding this comment.
It's duplicate but intentional. We want to get rid of IServerConnectionType and one approach is merging it into ServerUriStorage#onDidChangeConnectionType. I didn't do a clean up here as it will make this PR touch dozen more files.
|
|
||
| suite(`Remote Kernel Finder`, () => { | ||
| let disposables: Disposable[] = []; | ||
| let preferredRemoteKernelIdProvider: PreferredRemoteKernelIdProvider; | ||
| let remoteKernelFinder: RemoteKernelFinder; | ||
| let remoteKernelFinder: UniversalRemoteKernelFinder; |
There was a problem hiding this comment.
At the very end, we will drop the prefix Universal.
There was a problem hiding this comment.
Yeah, don't think that was a great name in the first place 😆 . Just needed something to distinguish it.
| @inject(IsWebExtension) private readonly isWebExtension: boolean | ||
| ) { | ||
| this._strategy = this.getStrategy(); | ||
| this.disposables.push(this._strategy); |
There was a problem hiding this comment.
Weird that there is no configurationService.onDidChange event.
Codecov Report
@@ Coverage Diff @@
## main #11879 +/- ##
=======================================
Coverage 63% 63%
=======================================
Files 490 490
Lines 34454 34607 +153
Branches 5590 5630 +40
=======================================
+ Hits 21752 21917 +165
+ Misses 10635 10608 -27
- Partials 2067 2082 +15
|
| remoteKernelFinder = new RemoteKernelFinder( | ||
| remoteKernelFinder = new UniversalRemoteKernelFinder( | ||
| 'currentremote', | ||
| 'Local Kernels', |
There was a problem hiding this comment.
This doesn't look correct here, plus the string should be localized if it's a display name.
There was a problem hiding this comment.
@IanMatthewHuff it's a test so I didn't try to make it localized.
There was a problem hiding this comment.
@rebornix was reading too fast. You are totally right :)
Merge Universal Remote Kerner Finder logic with legacy remote kernel finder.
package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed).