Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Sending Analytics Data for file Operation and LSP features #14683

Merged
merged 5 commits into from
Apr 9, 2019
Merged

Sending Analytics Data for file Operation and LSP features #14683

merged 5 commits into from
Apr 9, 2019

Conversation

niteskum
Copy link
Collaborator

@niteskum niteskum commented Apr 3, 2019


HealthLogger.sendAnalyticsData(
"usagelanguageChange" + filelanguageName + fileType,
"usage",
Copy link
Contributor

Choose a reason for hiding this comment

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

@niteskum can all analytics strings be moved to another file as variables and use that variable everywhere needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

moved all common strings constant to HealthLogger.js as earlier all constants were defined in this file

if (!docToSave) {
return;
}
var fileType = docToSave.language ? docToSave.language._name : "";
Copy link
Collaborator

Choose a reason for hiding this comment

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

@SnchitGrover Do we want an empty string here or something like "unknown"? Empty strings are sometimes treated as null values while serialization depending on platform. Depends on your use case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@niteskum Is there ever a possibility of not having the language property in a document object?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@shubhsnov I think document will always have language. but since file save is critical operation , don't want to leave any space in code to trigger exception so I am doing null check.

if(fileSize === -1) {
subType = "";
}
if(fileSize < 10) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't the check be <=? if the file size is 10kb, it will log in 10_50KB range.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@SnchitGrover
Copy link
Contributor

LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants