Activate banner prompt for Pylance #12817

MikhailArkhipov · 2020-07-08T21:21:36Z

Remove Pylance banner for Jedi users
100% Pylance banner for insiders and 50% for MPLS v1
Update tests
Change A/B experiments for Pylance to new API.
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).~~
~~- [ ] The wiki is updated with any design decisions/details.~~

src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts

src/client/languageServices/proposeLanguageServerBanner.ts

jakebailey

Marking this with request changes temporarily so we can verify the other side of the setting change. The actual banner and stuff is good.

jakebailey · 2020-07-15T23:17:04Z

I have write permission to this branch (as it's a PR), so I'm going to try and update it.

codecov-commenter · 2020-07-16T02:32:48Z

Codecov Report

❗ No coverage uploaded for pull request base (master@8687f01). Click here to learn what that means.
The diff coverage is 82.45%.

@@            Coverage Diff            @@
##             master   #12817   +/-   ##
=========================================
  Coverage          ?   59.91%           
=========================================
  Files             ?      671           
  Lines             ?    36649           
  Branches          ?     5154           
=========================================
  Hits              ?    21958           
  Misses            ?    13593           
  Partials          ?     1098

Impacted Files	Coverage Δ
...lient/common/application/applicationEnvironment.ts	`18.18% <0.00%> (ø)`
src/client/common/application/types.ts	`100.00% <ø> (ø)`
src/client/common/types.ts	`100.00% <ø> (ø)`
src/client/activation/none/activator.ts	`14.28% <37.50%> (ø)`
src/client/activation/languageServer/activator.ts	`93.93% <75.00%> (ø)`
...nt/languageServices/proposeLanguageServerBanner.ts	`93.47% <96.42%> (ø)`
src/client/activation/activationService.ts	`84.14% <100.00%> (ø)`
...ent/activation/node/languageServerFolderService.ts	`80.76% <100.00%> (ø)`
src/client/activation/serviceRegistry.ts	`82.27% <100.00%> (ø)`
src/client/common/constants.ts	`100.00% <100.00%> (ø)`
... and 674 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8687f01...c1de88a. Read the comment docs.

jakebailey · 2020-07-16T15:29:05Z

@kimadeline I've updated this, if you wanted to take another look with my extra changes.

kimadeline · 2020-07-16T15:30:21Z

Sounds good! I'll take a look

jakebailey

There's one other change that has to happen, unfortunately; this needs to use the (new) experiment system to allow the numbers to be tweaked without code changes. Marking as request changes until then. This can just wait for the next point release.

Sorry 🙁

kimadeline · 2020-07-16T17:54:58Z

Sounds good! I won't take a look just yet then lol

…nto exp

src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts

package.json

Old

jakebailey · 2020-07-21T17:55:15Z

The missing bit would be to not offer the banner if pylance is already installed.

MikhailArkhipov · 2020-07-21T21:03:47Z

        if (this.extensions.getExtension(PYLANCE_EXTENSION_ID)) {
            return false;
        }

jakebailey

I think this looks fine, but @kimadeline should look.

kimadeline · 2020-07-21T21:06:31Z

aight i'm on it

src/client/languageServices/proposeLanguageServerBanner.ts

src/client/common/constants.ts

sonarqubecloud · 2020-07-21T22:02:57Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

No Coverage information
0.3% Duplication

* Fix path * Actually fix settings * Add news * Add test * Format * Suppress 'jediEnabled' removal * Drop survey first launch threshold * Remove LS experiments * Frequency + tests * Fix test * Update message to match spec * Open workspace for extension rather than changing setting * Fix localization string * Show banners asynchronously * Add experiments * Formatting * Typo * Put back verifyAll * Remove obsolete experiments, add Pylance * Suppress experiment if Pylance is installed * PR feedback Co-authored-by: Jake Bailey <[email protected]>

* Ensure languageServer value is valid, send event during activate (#13064) * Update change log and version * Activate banner prompt for Pylance (#12817) * Fix path * Actually fix settings * Add news * Add test * Format * Suppress 'jediEnabled' removal * Drop survey first launch threshold * Remove LS experiments * Frequency + tests * Fix test * Update message to match spec * Open workspace for extension rather than changing setting * Fix localization string * Show banners asynchronously * Add experiments * Formatting * Typo * Put back verifyAll * Remove obsolete experiments, add Pylance * Suppress experiment if Pylance is installed * PR feedback Co-authored-by: Jake Bailey <[email protected]> * Update change log as per comments Co-authored-by: Jake Bailey <[email protected]> Co-authored-by: Mikhail Arkhipov <[email protected]>

* Update model.isTrusted on trust change (#12820) (#12823) * Reduce visual complexity of trust prompt (#12839) (#12847) * Port python 2.7 fix to release (#12877) * port color fix on collapse all (#12895) (#12897) * fix a color on collapse all (#12895) * update changelog * Merge fixes into July release (#12889) Co-authored-by: Timothy Ruscica <[email protected]> * Merge more fixes into july release (#12918) Co-authored-by: Joyce Er <[email protected]> * Port trust fixes (#12929) * Fix regressions in trusted notebooks (#12902) * Handle trustAllNotebooks selection * Fix bug where after trusting, UI didn't update * Recover from ENOENT due to missing parent directory when trusting notebook (#12913) * Disable keydown on native cells in untrusted notebooks (#12914) * Hide editor icons when editor is not a notebook (#12934) (#12935) * Check for hideFromUser before activating current terminal (#12942) (#12956) * Check for hideFromUser before activating current terminal * Add tests * Tweak logic * Port final trust fixes for release (#12965) * Only allow Enter / NumpadEnter w/o ctrl/shift/alt (#12939) * Send telemetry for notebook trust prompt selections (#12964) * Fixes for persisting trust (#12950) * Display survey for native notebooks on/after 1st August (#12961) (#12975) Co-authored-by: Joyce Er <[email protected]> Co-authored-by: Joyce Er <[email protected]> * Contains cherry picks, version updates, change log updates (#12983) * Update version and change log * Improve detection when LS is fully loaded for IntelliCode (#12853) * Fix path * Actually fix settings * Add news * Add test * Format * Suppress 'jediEnabled' removal * Drop survey first launch threshold * Wait for client ready * Handle async dispose * Fix the date Co-authored-by: Mikhail Arkhipov <[email protected]> * hide the gather button while a cell is executing (#12984) * Update date (#13002) * remove release notes from the start page (#13032) * Cherry pick, version change and change log update (#13079) * Ensure languageServer value is valid, send event during activate (#13064) * Update change log and version * Activate banner prompt for Pylance (#12817) * Fix path * Actually fix settings * Add news * Add test * Format * Suppress 'jediEnabled' removal * Drop survey first launch threshold * Remove LS experiments * Frequency + tests * Fix test * Update message to match spec * Open workspace for extension rather than changing setting * Fix localization string * Show banners asynchronously * Add experiments * Formatting * Typo * Put back verifyAll * Remove obsolete experiments, add Pylance * Suppress experiment if Pylance is installed * PR feedback Co-authored-by: Jake Bailey <[email protected]> * Update change log as per comments Co-authored-by: Jake Bailey <[email protected]> Co-authored-by: Mikhail Arkhipov <[email protected]> * Port fix the gather survey (#13086) (#13105) * Fix the gather survey (#13086) * fix the gather survey added 'gather stats' telemetry mention the gather comments to update the python ext * oops * fix tests and address comments * update gather stats when resetting the kernel * set globalstate vars to 0 when we open vs code * fix gather stats telemetry * fix tests * fix tests for real Co-authored-by: Joyce Er <[email protected]> Co-authored-by: Ian Huff <[email protected]> Co-authored-by: David Kutugata <[email protected]> Co-authored-by: Don Jayamanne <[email protected]> Co-authored-by: Timothy Ruscica <[email protected]> Co-authored-by: Mikhail Arkhipov <[email protected]> Co-authored-by: Jake Bailey <[email protected]>

Mikhail Arkhipov added 17 commits May 5, 2020 21:15

Fix path

04e0b34

Merge branch 'master' of https://github.com/Microsoft/vscode-python

9297d96

Merge branch 'master' of https://github.com/Microsoft/vscode-python

9f3d5ac

Merge branch 'master' of https://github.com/Microsoft/vscode-python

607bf98

Merge branch 'master' of https://github.com/Microsoft/vscode-python

e566214

Actually fix settings

4477900

Add news

d0d50de

Add test

03aa5f9

Format

e4a032f

Suppress 'jediEnabled' removal

27eeec7

Merge branch 'master' of https://github.com/Microsoft/vscode-python

89f432e

Drop survey first launch threshold

3ed655b

Merge branch 'master' of https://github.com/Microsoft/vscode-python

64ac38a

Merge branch 'master' of https://github.com/Microsoft/vscode-python

52a4325

Merge branch 'master' of https://github.com/Microsoft/vscode-python

67d7cb8

Remove LS experiments

bbe5a7f

Frequency + tests

f29f7d7

MikhailArkhipov added the no-changelog No news entry required label Jul 8, 2020

Fix test

19ccc20

MikhailArkhipov marked this pull request as ready for review July 9, 2020 03:45

karthiknadig requested review from kimadeline, karthiknadig and DonJayamanne July 14, 2020 20:52

kimadeline reviewed Jul 15, 2020

View reviewed changes

src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts Outdated Show resolved Hide resolved

jakebailey reviewed Jul 15, 2020

View reviewed changes

src/client/languageServices/proposeLanguageServerBanner.ts Outdated Show resolved Hide resolved

kimadeline approved these changes Jul 15, 2020

View reviewed changes

jakebailey previously requested changes Jul 15, 2020

View reviewed changes

Update message to match spec

4dec978

Open workspace for extension rather than changing setting

a7fe6a8

kimadeline self-requested a review July 16, 2020 15:30

jakebailey previously requested changes Jul 16, 2020

View reviewed changes

Mikhail Arkhipov added 4 commits July 20, 2020 11:24

Merge branch 'master' of https://github.com/Microsoft/vscode-python i…

1396abd

…nto exp

Add experiments

2450fc7

Formatting

0300726

Typo

f0d528c

jakebailey reviewed Jul 20, 2020

View reviewed changes

src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts Show resolved Hide resolved

Mikhail Arkhipov added 2 commits July 20, 2020 16:08

Put back verifyAll

57de3f8

Remove obsolete experiments, add Pylance

b3c85fc

jakebailey reviewed Jul 21, 2020

View reviewed changes

package.json Show resolved Hide resolved

Suppress experiment if Pylance is installed

1ca6b11

jakebailey approved these changes Jul 21, 2020

View reviewed changes

kimadeline suggested changes Jul 21, 2020

View reviewed changes

src/client/languageServices/proposeLanguageServerBanner.ts Outdated Show resolved Hide resolved

src/client/common/constants.ts Outdated Show resolved Hide resolved

PR feedback

c1de88a

kimadeline self-requested a review July 21, 2020 23:34

kimadeline approved these changes Jul 21, 2020

View reviewed changes

MikhailArkhipov merged commit 5e884b1 into microsoft:master Jul 22, 2020

MikhailArkhipov deleted the exp branch July 30, 2020 22:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Activate banner prompt for Pylance #12817

Activate banner prompt for Pylance #12817

Uh oh!

MikhailArkhipov commented Jul 8, 2020 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

jakebailey left a comment

Uh oh!

jakebailey commented Jul 15, 2020

Uh oh!

codecov-commenter commented Jul 16, 2020 •

edited

Loading

Uh oh!

jakebailey commented Jul 16, 2020

Uh oh!

kimadeline commented Jul 16, 2020

Uh oh!

jakebailey left a comment

Uh oh!

kimadeline commented Jul 16, 2020

Uh oh!

Uh oh!

Uh oh!

jakebailey commented Jul 21, 2020

Uh oh!

MikhailArkhipov commented Jul 21, 2020

Uh oh!

jakebailey left a comment

Uh oh!

kimadeline commented Jul 21, 2020

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Jul 21, 2020

Uh oh!

Uh oh!

Activate banner prompt for Pylance #12817

Activate banner prompt for Pylance #12817

Uh oh!

Conversation

MikhailArkhipov commented Jul 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jakebailey left a comment

Choose a reason for hiding this comment

Uh oh!

jakebailey commented Jul 15, 2020

Uh oh!

codecov-commenter commented Jul 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jakebailey commented Jul 16, 2020

Uh oh!

kimadeline commented Jul 16, 2020

Uh oh!

jakebailey left a comment

Choose a reason for hiding this comment

Uh oh!

kimadeline commented Jul 16, 2020

Uh oh!

Uh oh!

Uh oh!

jakebailey commented Jul 21, 2020

Uh oh!

MikhailArkhipov commented Jul 21, 2020

Uh oh!

jakebailey left a comment

Choose a reason for hiding this comment

Uh oh!

kimadeline commented Jul 21, 2020

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Jul 21, 2020

Uh oh!

Uh oh!

MikhailArkhipov commented Jul 8, 2020 •

edited

Loading

codecov-commenter commented Jul 16, 2020 •

edited

Loading