Activate banner prompt for Pylance by MikhailArkhipov · Pull Request #12817 · microsoft/vscode-python

Mikhail Arkhipov (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.~~

Jake Bailey (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.

Jake Bailey (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 Comments Bot (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.

Jake Bailey (jakebailey) · 2020-07-16T15:29:05Z

Kim-Adeline Miguel (@kimadeline) I've updated this, if you wanted to take another look with my extra changes.

Kim-Adeline Miguel (kimadeline) · 2020-07-16T15:30:21Z

Sounds good! I'll take a look

Jake Bailey (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 🙁

Kim-Adeline Miguel (kimadeline) · 2020-07-16T17:54:58Z

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

…nto exp

Old

Jake Bailey (jakebailey) · 2020-07-21T17:55:15Z

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

Mikhail Arkhipov (MikhailArkhipov) · 2020-07-21T21:03:47Z

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

Jake Bailey (jakebailey)

I think this looks fine, but Kim-Adeline Miguel (@kimadeline) should look.

Kim-Adeline Miguel (kimadeline) · 2020-07-21T21:06:31Z

aight i'm on it

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 <5341706+jakebailey@users.noreply.github.com>

* 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 <5341706+jakebailey@users.noreply.github.com> * Update change log as per comments Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Co-authored-by: Mikhail Arkhipov <mikhaila@microsoft.com>

* 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 <35348871+techwithtim@users.noreply.github.com> * Merge more fixes into july release (#12918) Co-authored-by: Joyce Er <joyceerhuiling@gmail.com> * 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 <joyceerhuiling@gmail.com> Co-authored-by: Joyce Er <joyceerhuiling@gmail.com> * 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 <mikhaila@microsoft.com> * 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 <5341706+jakebailey@users.noreply.github.com> * Update change log as per comments Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Co-authored-by: Mikhail Arkhipov <mikhaila@microsoft.com> * 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 <joyceerhuiling@gmail.com> Co-authored-by: Ian Huff <ianhu@microsoft.com> Co-authored-by: David Kutugata <dakutuga@microsoft.com> Co-authored-by: Don Jayamanne <don.jayamanne@yahoo.com> Co-authored-by: Timothy Ruscica <35348871+techwithtim@users.noreply.github.com> Co-authored-by: Mikhail Arkhipov <mikhaila@microsoft.com> Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com>

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

Mikhail Arkhipov (MikhailArkhipov) added the no-changelog No news entry required label Jul 8, 2020

Fix test

19ccc20

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

Karthik Nadig (karthiknadig) requested review from Don Jayamanne (DonJayamanne), Karthik Nadig (karthiknadig) and Kim-Adeline Miguel (kimadeline) July 14, 2020 20:52

Kim-Adeline Miguel (kimadeline) reviewed Jul 15, 2020

View reviewed changes

Comment thread src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts Outdated

Jake Bailey (jakebailey) reviewed Jul 15, 2020

View reviewed changes

Comment thread src/client/languageServices/proposeLanguageServerBanner.ts Outdated

Kim-Adeline Miguel (kimadeline) approved these changes Jul 15, 2020

View reviewed changes

Jake Bailey (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

Kim-Adeline Miguel (kimadeline) self-requested a review July 16, 2020 15:30

Jake Bailey (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

Jake Bailey (jakebailey) reviewed Jul 20, 2020

View reviewed changes

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

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

Put back verifyAll

57de3f8

Remove obsolete experiments, add Pylance

b3c85fc

Jake Bailey (jakebailey) reviewed Jul 21, 2020

View reviewed changes

Comment thread package.json

Suppress experiment if Pylance is installed

1ca6b11

Jake Bailey (jakebailey) approved these changes Jul 21, 2020

View reviewed changes

Kim-Adeline Miguel (kimadeline) suggested changes Jul 21, 2020

View reviewed changes

Comment thread src/client/languageServices/proposeLanguageServerBanner.ts Outdated

Comment thread src/client/common/constants.ts Outdated

PR feedback

c1de88a

Kim-Adeline Miguel (kimadeline) self-requested a review July 21, 2020 23:34

Kim-Adeline Miguel (kimadeline) approved these changes Jul 21, 2020

View reviewed changes

Mikhail Arkhipov (MikhailArkhipov) merged commit 5e884b1 into microsoft:master Jul 22, 2020

Mikhail Arkhipov (MikhailArkhipov) deleted the exp branch July 30, 2020 22:28

Conversation

Mikhail Arkhipov (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!

Jake Bailey (jakebailey) left a comment

Choose a reason for hiding this comment

Uh oh!

Jake Bailey (jakebailey) commented Jul 15, 2020

Uh oh!

Codecov Comments Bot (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!

Jake Bailey (jakebailey) commented Jul 16, 2020

Uh oh!

Kim-Adeline Miguel (kimadeline) commented Jul 16, 2020

Uh oh!

Jake Bailey (jakebailey) left a comment

Choose a reason for hiding this comment

Uh oh!

Kim-Adeline Miguel (kimadeline) commented Jul 16, 2020

Uh oh!

Uh oh!

Uh oh!

Jake Bailey (jakebailey) commented Jul 21, 2020

Uh oh!

Mikhail Arkhipov (MikhailArkhipov) commented Jul 21, 2020

Uh oh!

Jake Bailey (jakebailey) left a comment

Choose a reason for hiding this comment

Uh oh!

Kim-Adeline Miguel (kimadeline) commented Jul 21, 2020

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Jul 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Mikhail Arkhipov (MikhailArkhipov) commented Jul 8, 2020 •

edited

Loading

Codecov Comments Bot (codecov-commenter) commented Jul 16, 2020 •

edited

Loading