Skip to content

Dev/greazer/telem updates#10631

Merged
greazer merged 8 commits into
masterfrom
dev/greazer/telem-updates
Mar 18, 2020
Merged

Dev/greazer/telem updates#10631
greazer merged 8 commits into
masterfrom
dev/greazer/telem-updates

Conversation

@greazer

@greazer greazer commented Mar 17, 2020

Copy link
Copy Markdown
Member
  • Add some additional telemetry to track gather use.
  • Updated the header at the top of gathered notebooks and script to point to a survey.

@greazer greazer added the no-changelog No news entry required label Mar 17, 2020
await this.ipynbProvider.createNew(contents);
const editor = await this.ipynbProvider.createNew(contents);
this.gatheredNotebookList.push(editor);
editor.saved(this.onSavedGatheredNotebook.bind(this));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You could do this another way.

editor.saved returns an idisposable. So you could eliminate the need for saving the editors in the list. Something like so:

let disposable : IDisposable;
const handler = () => {
   sendTelemetryEvent(Telemetry.GatherSaved);
   if (disposable) {
      disposable.dispose();
   }
}
disposable = editor.saved(handler);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, nice. I figured there had to be a prettier way

@rchiodo rchiodo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

:shipit:

@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #10631 into master will decrease coverage by 0.17%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10631      +/-   ##
==========================================
- Coverage   60.79%   60.62%   -0.18%     
==========================================
  Files         579      583       +4     
  Lines       31403    31551     +148     
  Branches     4469     4487      +18     
==========================================
+ Hits        19091    19127      +36     
- Misses      11342    11450     +108     
- Partials      970      974       +4     
Impacted Files Coverage Δ
src/client/common/utils/localize.ts 95.19% <ø> (ø)
src/client/telemetry/index.ts 87.38% <ø> (ø)
src/client/datascience/constants.ts 99.71% <100.00%> (+<0.01%) ⬆️
src/client/datascience/gather/gather.ts 34.84% <100.00%> (+2.03%) ⬆️
src/datascience-ui/react-common/arePathsSame.ts 75.00% <0.00%> (-12.50%) ⬇️
src/client/common/utils/platform.ts 64.70% <0.00%> (-11.77%) ⬇️
...t/common/insidersBuild/insidersExtensionService.ts 97.18% <0.00%> (-2.82%) ⬇️
src/client/linters/pydocstyle.ts 86.66% <0.00%> (-2.23%) ⬇️
src/client/datascience/debugLocationTracker.ts 76.56% <0.00%> (-1.57%) ⬇️
...ascience/interactive-ipynb/nativeEditorProvider.ts 52.24% <0.00%> (-1.13%) ⬇️
... and 22 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 dbf4d70...a6dcd48. Read the comment docs.

@sonarqubecloud

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@greazer

greazer commented Mar 18, 2020

Copy link
Copy Markdown
Member Author

Made suggested change by Rich before submitting.

@greazer greazer merged commit 7f24ef7 into master Mar 18, 2020
DonJayamanne added a commit that referenced this pull request Mar 20, 2020
* master:
  Turning new debugger experiment on for all users and reload experiment for 20% of users (#10674)
  Disable blank issues (#10678)
  update mockito and minor test updates (#10671)
  Fix save on old editor (#10665)
  Job names were invalid (#10646)
  Telemetry updates (#10631)
  Remote session shutdownAll is killing all sessions (#10621)
DonJayamanne added a commit that referenced this pull request Mar 20, 2020
* master:
  Turning new debugger experiment on for all users and reload experiment for 20% of users (#10674)
  Disable blank issues (#10678)
  update mockito and minor test updates (#10671)
  Fix save on old editor (#10665)
  Job names were invalid (#10646)
  Telemetry updates (#10631)
  Remote session shutdownAll is killing all sessions (#10621)
@lock lock Bot locked as resolved and limited conversation to collaborators Mar 27, 2020
@greazer greazer deleted the dev/greazer/telem-updates branch March 28, 2020 09:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

no-changelog No news entry required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants