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

Port surveys #4804

Merged
merged 3 commits into from
Feb 16, 2021
Merged

Port surveys #4804

merged 3 commits into from
Feb 16, 2021

Conversation

DavidKutu
Copy link

For #4726

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

David Kutugata added 2 commits February 12, 2021 18:22
* Refactor surveys and add a new one

* add news and lint

* lint

* lint

* PR comments
@DavidKutu DavidKutu requested a review from a team as a code owner February 13, 2021 02:26
@codecov-io
Copy link

Codecov Report

Merging #4804 (b1e9840) into release-2021.02 (43efcda) will decrease coverage by 0%.
The diff coverage is 66%.

@@               Coverage Diff                @@
##           release-2021.02   #4804    +/-   ##
================================================
- Coverage               73%     72%    -1%     
================================================
  Files                  411     410     -1     
  Lines                26960   26942    -18     
  Branches              3907    3903     -4     
================================================
- Hits                 19882   19660   -222     
- Misses                5489    5703   +214     
+ Partials              1589    1579    -10     
Impacted Files Coverage Δ
src/client/common/types.ts 100% <ø> (ø)
src/client/datascience/serviceRegistry.ts 98% <ø> (-1%) ⬇️
src/client/datascience/dataScienceSurveyBanner.ts 65% <65%> (-24%) ⬇️
src/client/activation/serviceRegistry.ts 100% <100%> (ø)
src/client/datascience/notebook/introStartPage.ts 83% <100%> (ø)
src/client/datascience/shiftEnterBanner.ts 91% <100%> (ø)
...t/common/application/webviewPanels/webviewPanel.ts 10% <0%> (-63%) ⬇️
src/client/common/application/webviews/webview.ts 13% <0%> (-54%) ⬇️
.../application/webviewPanels/webviewPanelProvider.ts 43% <0%> (-50%) ⬇️
...nt/datascience/interactive-common/debugListener.ts 34% <0%> (-40%) ⬇️
... and 35 more

1. Minimize number of icons on the notebook toolbar (put the rest in overflow).
([#4730](https://github.com/Microsoft/vscode-jupyter/issues/4730))
1. Add survey for the new Notebooks experience experiment.
([#4726](https://github.com/microsoft/vscode-jupyter/issues/4726))
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we already had a survey for native notebooks & #4726 was just an update to the uri.
Or am i mistaken?

Copy link
Author

Choose a reason for hiding this comment

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

Its a different survey, the old one is for native notebooks in insiders, this one is for the experiment. We want to have both.

@rchiodo
Copy link
Contributor

rchiodo commented Feb 16, 2021

Smoke tests don't seem to be passing. I think you need to fix this part too:

DataScience Error Error: No matching bindings found for serviceIdentifier: Symbol(IJupyterExtensionBanner)
 Symbol(IJupyterExtensionBanner) - named: DSSurveyBanner

Meaning port Joyce's fix once it goes through as well.

Copy link
Contributor

@rchiodo rchiodo left a comment

Choose a reason for hiding this comment

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

⏲️

@DavidKutu DavidKutu requested a review from rchiodo February 16, 2021 20:06
@DavidKutu DavidKutu merged commit f5e272d into release-2021.02 Feb 16, 2021
@DavidKutu DavidKutu deleted the david/PortSurveys branch February 16, 2021 20:31
rchiodo pushed a commit that referenced this pull request Feb 18, 2021
* Refactor surveys and add a new one (#4794)

* Refactor surveys and add a new one

* add news and lint

* lint

* lint

* PR comments

* update changelog

* Fix IJupyterExtensionBanner service identifier (#4825)

Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
rchiodo added a commit that referenced this pull request Feb 18, 2021
* PORT Variable view icon to release branch (#4748)

* update changelog (#4752)

* Cherry pick change for icon overflow (#4753)

* Change number of icons in notebook toolbar (#4747)

* Fix icons

* Have trust command show up and nothing else if untrusted

* Add trust check to all commands

* Update based on discussion

* Update change log

* PORT - variable context (#4770)

* Add new editor context for just if native notebook is active (#4762)

* update news

Co-authored-by: Ian Huff <ianhuff@LICHYOGAIOT.northamerica.corp.microsoft.com>

* Port control enter fix to release (#4774)

* Fix ctrl+enter and alt+enter by removing the redundant command. (#4769)

* Fix ctrl+enter and alt+enter by removing the redundant command.

* Code review feedback

* Update changelog

* port .NET fix to release (#4776)

* Fix .NET interactive to always use native (#4772)

* Fix .NET interactive

* Fix tests and linter

* Update changelog

* Port kernel finder change to release (#4799)

* Fix duplicate kernel generation (#4790)

* Fix duplicate kernel generation

* Fix functional tests

* Update changelog

* Actually use stable

* PORT - Port document dispose fix to release. (#4798)

* Only clean up editors on close not on Panel dispose (#4787)

* update changelog

Co-authored-by: Ian Huff <ianhuff@LICHYOGAIOT.northamerica.corp.microsoft.com>

* Port surveys (#4804)

* Refactor surveys and add a new one (#4794)

* Refactor surveys and add a new one

* add news and lint

* lint

* lint

* PR comments

* update changelog

* Fix IJupyterExtensionBanner service identifier (#4825)

Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>

* PORT - Fix for top level save and undo commands (#4851)

* Update cache ignore

* Actually ignore .so files

* Update release plan

* Put back package.json changes

* Delete news entries

Co-authored-by: Ian Huff <ianhu@microsoft.com>
Co-authored-by: David Kutugata <dakutuga@microsoft.com>
Co-authored-by: Ian Huff <ianhuff@LICHYOGAIOT.northamerica.corp.microsoft.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
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.

5 participants