Use the same "Jupyter is not installed" message everywhere - #16372
Merged
Kim-Adeline Miguel (kimadeline) merged 3 commits intoJun 7, 2021
Conversation
Kim-Adeline Miguel (kimadeline)
added
no-changelog
| ) {} | ||
| constructor(@inject(IExtensions) private extensions: IExtensions) {} | ||
|
|
||
| public get isJupyterExtensionInstalled(): boolean { |
There was a problem hiding this comment.
isJupyterExtensionInstalled still lives here instead of being moved to JupyterNotInstalledNotificationHelper because the latter has one single purpose and it's the notification, while a public-facing way of checking whether the Jupyter extension is installed is a separate responsibility.
Kim-Adeline Miguel (kimadeline)
marked this pull request as ready for review
June 2, 2021 19:50
github-actions
Bot
requested review from
Karthik Nadig (karthiknadig) and
Paula (paulacamargo25)
June 2, 2021 19:50
Karthik Nadig (karthiknadig)
approved these changes
Jun 2, 2021
Paula (paulacamargo25)
approved these changes
Jun 7, 2021
Kartik Raj (karrtikr)
approved these changes
Jun 7, 2021
Kartik Raj (karrtikr)
left a comment
Kartik Raj (karrtikr)
left a comment
There was a problem hiding this comment.
LGTM other than this clarification.
Author
|
Insiders failure not related to this PR, merging it. |
Kim-Adeline Miguel (kimadeline)
merged commit Jun 7, 2021
b8997c8
into
microsoft:16102-jupyter-dependency
Kim-Adeline Miguel (kimadeline)
deleted the
16102-standardize-not-installed-message
branch
June 7, 2021 21:27
Kartik Raj (karrtikr)
pushed a commit
that referenced
this pull request
Jun 8, 2021
* Make Jupyter an optional dependency (#16267) * News entry * Move Jupyter to the optional dependencies step * Update news/1 Enhancements/16102.md Co-authored-by: Kartik Raj <karraj@microsoft.com> Co-authored-by: Kartik Raj <karraj@microsoft.com> * License wording update (#16278) * Wording * License wording * Add a "Jupyter not installed" notification helper (#16321) * Add telemetry info * Use enum for the telemetry * Add prompt as a standalone function * Remove "Install" from the prompt * Make it a class * Register singleton * Rename file to a long but descriptive name * Unit tests * Add to package.nls.json * Use sinon for tests * Use the same "Jupyter is not installed" message everywhere (#16372) * rename to showJupyterNotInstalledPrompt * Replace existing prompt with new prompt * Remove Jupyter check from command manager * Update the start page to use the prompt (#16417) * Update copy * Update origin key * Show prompt if jupyter not installed & should show * Add tests for this functionality only * Update news entry * Remove comments * follow-up from the merge * Add singletons for startpage functional tests * Missing one symbol * Update src/client/common/startPage/startPage.ts Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com> * Add logging Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com> Co-authored-by: Kartik Raj <karraj@microsoft.com> Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For #16102
This PR does 2 things:
jupyterNotInstalledPrompt->showJupyterNotInstalledPromptinsrc/client/jupyter/jupyterNotInstalledNotificationHelper.tsinstallJupyterExtensionfromsrc/client/jupyter/jupyterExtensionDependencyManager.ts