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

Display notebook paths in kernel picker for remote kernels #8181

Merged
merged 3 commits into from
Nov 5, 2021

Conversation

DonJayamanne
Copy link
Contributor

For #8175

@DonJayamanne DonJayamanne requested a review from a team as a code owner November 5, 2021 17:05
@rchiodo
Copy link
Contributor

rchiodo commented Nov 5, 2021

What does it look like?

@DonJayamanne
Copy link
Contributor Author

@rchiodo the screenshot is here #8175 (comment)
Wait will upload here too so we can review & discuss
image

@rchiodo
Copy link
Contributor

rchiodo commented Nov 5, 2021

I think you should switch it.

The details should be the 'last activity, 1 existing connections' or maybe not there altogether.
The descriptoin? field (thing to right of name) should be the notebook path. It's the most important piece of information.

Copy link
Member

@IanMatthewHuff IanMatthewHuff left a comment

Choose a reason for hiding this comment

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

Agree with Rich's comment on order switch, but looks good.

@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2021

Codecov Report

Merging #8181 (6318383) into main (1793f34) will increase coverage by 0%.
The diff coverage is 40%.

@@          Coverage Diff          @@
##            main   #8181   +/-   ##
=====================================
  Coverage     71%     71%           
=====================================
  Files        370     370           
  Lines      22598   22600    +2     
  Branches    3419    3420    +1     
=====================================
+ Hits       16222   16226    +4     
  Misses      4982    4982           
+ Partials    1394    1392    -2     
Impacted Files Coverage Δ
src/client/datascience/jupyter/kernels/helpers.ts 71% <0%> (-1%) ⬇️
src/client/datascience/jupyter/kernels/types.ts 100% <ø> (ø)
...atascience/notebook/kernelFilter/kernelFilterUI.ts 30% <ø> (ø)
...t/datascience/notebook/vscodeNotebookController.ts 76% <100%> (+<1%) ⬆️
src/client/datascience/raw-kernel/rawSocket.ts 82% <0%> (-2%) ⬇️
src/client/debugger/jupyter/kernelDebugAdapter.ts 82% <0%> (+<1%) ⬆️
src/client/datascience/jupyter/kernelVariables.ts 57% <0%> (+1%) ⬆️
.../datascience/editor-integration/codeLensFactory.ts 89% <0%> (+1%) ⬆️

@DonJayamanne
Copy link
Contributor Author

Done
image

@DonJayamanne DonJayamanne merged commit 1247ee6 into main Nov 5, 2021
@DonJayamanne DonJayamanne deleted the fixIssue8175 branch November 5, 2021 19: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.

4 participants