Bug 1694770 - Implement the timespan metric type #253
Conversation
Dexterp37
left a comment
Dexterp37
left a comment
There was a problem hiding this comment.
Looks good overall, there's a few nits and a CHANGELOG entry should be added as well. Does the glean_parser already produce valid code to use this?
Yes, we were only missing labeled. Unless we introduce a metric type that the Glean SDK does not also support, we are good. |
brizental
force-pushed
the
1694770-timespan
branch
from
b689d9e to
4dbef01
Compare
| * No error is recorded if no `start()` was called. | ||
| */ | ||
| cancel(): void { | ||
| Context.dispatcher.launch(() => { |
There was a problem hiding this comment.
Why wasn't this failing our tests? did you end up finding out?
There was a problem hiding this comment.
Dispatched or not, start and cancel when called in sequence seem to run one after the other. Probably because neither have any actual await inside. I tried to make the tests fail, but they never did.
Only when I launch some setTimeout's in between calling them do they fail, but I did not commit such a test because I thought it would be confusing.
2 participants
No description provided.