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 "More Info" option to ipykernel installer prompt + wiki link to provide more info #10845

Merged
merged 5 commits into from
Jul 25, 2022

Conversation

IanMatthewHuff
Copy link
Member

@IanMatthewHuff IanMatthewHuff commented Jul 18, 2022

Fixes #10658

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


// Link to our wiki page on jupyter kernels + ipykernel
// https://github.com/microsoft/vscode-jupyter/wiki/Jupyter-Kernels-and-the-Jupyter-Extension#python-extension-and-ipykernel
this.appShell.openUrl('https://aka.ms/AAhi594');
Copy link
Member Author

Choose a reason for hiding this comment

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

this.appShell.openUrl('https://aka.ms/AAhi594');
}
// "More Info" isn't a full valid response here, so reprompt after showing it
} while (selection === moreInfoOption);
Copy link
Member Author

@IanMatthewHuff IanMatthewHuff Jul 18, 2022

Choose a reason for hiding this comment

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

Loop here felt a bit funky, but I don't believe there is a way to keep open a modal dialog and provide the button selection. Returning back a KernelDependencyResponse of "moreInfo" didn't seem right, so just ask again until we get a valid response (cancel, install, selectKernel).

@IanMatthewHuff IanMatthewHuff marked this pull request as ready for review July 18, 2022 11:27
@IanMatthewHuff IanMatthewHuff requested a review from a team as a code owner July 18, 2022 11:27
@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2022

Codecov Report

Merging #10845 (7102658) into main (19e19a4) will increase coverage by 0%.
The diff coverage is 71%.

@@          Coverage Diff           @@
##            main   #10845   +/-   ##
======================================
  Coverage     63%      63%           
======================================
  Files        481      482    +1     
  Lines      33625    33695   +70     
  Branches    5487     5502   +15     
======================================
+ Hits       21285    21362   +77     
- Misses     10292    10293    +1     
+ Partials    2048     2040    -8     
Impacted Files Coverage Δ
src/telemetry.ts 100% <ø> (ø)
src/kernels/kernelDependencyService.node.ts 84% <71%> (-2%) ⬇️
...lPythonAndRelatedNonPythonKernelSpecFinder.node.ts 58% <0%> (-13%) ⬇️
src/kernels/jupyter/remoteKernelFinder.ts 88% <0%> (-4%) ⬇️
.../standalone/survey/dataScienceSurveyBanner.node.ts 66% <0%> (-4%) ⬇️
src/platform/errors/index.ts 65% <0%> (-4%) ⬇️
src/kernels/kernelFinder.base.ts 80% <0%> (-4%) ⬇️
...c/platform/interpreter/interpreterPackages.node.ts 76% <0%> (-3%) ⬇️
src/kernels/jupyter/baseKernelConnectionWrapper.ts 30% <0%> (-3%) ⬇️
.../notebooks/controllers/vscodeNotebookController.ts 80% <0%> (-2%) ⬇️
... and 32 more

@IanMatthewHuff IanMatthewHuff merged commit 5682a86 into main Jul 25, 2022
@IanMatthewHuff IanMatthewHuff deleted the dev/ianhu/ipykernelInfo branch July 25, 2022 19:55
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.

Consider adding a More Info button to the modal dialog prompt requiring users to install ipykernel
5 participants