Conversation
|
|
||
| if (initializationPromise) { | ||
| try { | ||
| // Use 'then' instead of 'await' because telemetry should be "fire and forget". |
There was a problem hiding this comment.
logDebuggerEvent and logLanguageServerEvent were already "fire and forget" due to not awaiting calls to it. We don't use no-floating-promises. There are many cases in which async functions are not being awaited, and cannot be awaited (such as when registered as a callback to a registerCommand or when registering for various VS Code events. There are other intentional cases a well). Are you sure you want to take this change an incur more calls to then?
There was a problem hiding this comment.
My intention was to keep the two implementations in sync. If you don't want this part of the change I can back it out. There's nothing wrong with using .then for intentional "fire and forget" cases. We still have several .then calls in the extension.
|
@bobbrow Are you going to check this in now? |
|
@bobbrow Did you want to check this in? |
|
Sorry, I thought you were going to merge this for me. |
|
Thanks @Colengms. I had updated the branch, and then forgot again. |
3 participants
While reviewing the exp platform changes for CMake Tools, a few issues were noticed that we wanted to bring fixes back to cpptools for:
getExperimentationServicereturning undefinedexperimentationTelemetryshould always be disposedlog*Event.