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
add owner and comment to a telemetry event #148752
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Owner is in the wrong spot in a few places, no need for @
just github usernames comma separated. Top level comments are needed too 👍
src/vs/workbench/services/languageDetection/common/languageDetectionWorkerService.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/services/languageDetection/common/languageDetectionWorkerService.ts
Outdated
Show resolved
Hide resolved
@lramos15 ok hopefully I got it right now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot a few at the bottom, others look good 👍
src/vs/workbench/services/languageDetection/common/languageDetectionWorkerService.ts
Outdated
Show resolved
Hide resolved
gah ok there we go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Properties should have unique comments that are not duplicates of the event. We want a living ledger that provides verbose information regarding the events and all their properties. If the comments are duplicated then they feel unnecessary and as if they add no value.
src/vs/workbench/services/languageDetection/common/languageDetectionWorkerService.ts
Outdated
Show resolved
Hide resolved
Ok @lramos15 how is this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still one left with duplicated comments. Sorry for all the back and forth I appreciate it!
hopefully we're good here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks so much for completing this 🎉
No description provided.