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.

Sign up for GitHub

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

Jump to bottom

Rename telemetry events #453

Merged
merged 1 commit into from
Apr 16, 2021
Merged

Rename telemetry events #453

merged 1 commit into from
Apr 16, 2021

Conversation

datho7561
Copy link
Contributor

@datho7561 datho7561 commented Apr 14, 2021 •
edited
Loading

Use . consistently as the separator in event names. Prefix event names with xml.. Group the binary download events into xml_binary_download. Add a parameter status that indicates if the download succeeded, failed or was aborted.

Signed-off-by: David Thompson davthomp@redhat.com

@datho7561
Copy link
Contributor Author

@fbricon

datho7561
datho7561 commented Apr 14, 2021
View reviewed changes
src/server/binary/binaryServerStarter.ts Outdated Show resolved Hide resolved
src/server/binary/binaryServerStarter.ts Outdated Show resolved Hide resolved
@datho7561 datho7561 force-pushed the change-download-event-name branch from 61805c6 to 8fa73cb Compare April 14, 2021 17:14
@fbricon
Copy link
Collaborator

fbricon commented Apr 14, 2021 •
edited
Loading

Please use . instead of _ as separator

fbricon
fbricon reviewed Apr 14, 2021
View reviewed changes
src/telemetry.ts Outdated Show resolved Hide resolved
fbricon
fbricon reviewed Apr 14, 2021
View reviewed changes
src/telemetry.ts Outdated Show resolved Hide resolved
@datho7561
Copy link
Contributor Author

Please use . instead of _ as separator

I thought that there was some issue with using .?

@datho7561 datho7561 force-pushed the change-download-event-name branch from 8fa73cb to d6c4e32 Compare April 14, 2021 17:36
@fbricon
Copy link
Collaborator

fbricon commented Apr 14, 2021 •
edited
Loading

I thought that there was some issue with using .?

No the issue is with :. Using . is consistent with the rest of our existing events

@datho7561
Copy link
Contributor Author

No the issue is with :. Using . is consistent with the rest of our existing events

Okay. LemMinX's reporting is also inconsistent then (lemminx uses server_start, server_stop). I'll take a look at what needs to be done to change it in LemMinX as well.

@datho7561
Rename telemetry events
cf4ed6a 
Use `.` consistently as the separator in event names.
Prefix event names with `xml.`.
Group the binary download events into `xml_binary_download`.
Add a parameter `status` that indicates if the download succeeded, failed or was aborted.

Signed-off-by: David Thompson <davthomp@redhat.com>
@datho7561 datho7561 force-pushed the change-download-event-name branch from d6c4e32 to cf4ed6a Compare April 14, 2021 19:03
@datho7561 datho7561 changed the title Change the names of the telemetry events Rename telemetry events Apr 14, 2021
@datho7561 datho7561 marked this pull request as ready for review April 14, 2021 19:11
@datho7561 datho7561 modified the milestone: 0.17.0 Apr 14, 2021
@datho7561
Copy link
Contributor Author

eclipse-lemminx/lemminx#1017

@datho7561 datho7561 requested a review from fbricon April 16, 2021 14:27
@fbricon fbricon merged commit 458f7e9 into redhat-developer:master Apr 16, 2021
@datho7561 datho7561 deleted the change-download-event-name branch April 16, 2021 17:59
@datho7561 datho7561 added this to the 0.17.0 milestone Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fbricon fbricon fbricon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.16.1
Development

Successfully merging this pull request may close these issues.
2 participants
@datho7561 @fbricon