Consider whether FOG's Datetime should return something better than a String in C++ and JS
Categories
(Toolkit :: Telemetry, task, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox90 | --- | fixed |
People
(Reporter: chutten, Assigned: chutten)
References
(Blocks 1 open bug)
Details
(Whiteboard: [telemetry:fog:m?])
Attachments
(3 files)
The MLA languages (C++ and JS) return an ISO8601-formatted string value for testGetValue
which, though precise, requires parsing to be useful to users writing instrumentation tests.
Since the APIs take better types than string (PRExplodedTime
for C++, Date
for JS), maybe the test API should return such a better type, too?
This bug is for considering the ramifications of this (FFI-compatible returned data type, what does a JS date look like as a jsval
xpidl type, is the API support for checking string values actually better than the API support for checking these 'better' types, etc) and implementing that decision (either by documenting the rationale in The Book or by changing the behaviour (and then documenting the changed behaviour in The Book)).
Comment 1•3 years ago
|
||
The MLA languages (C++ and JS) return an ISO8601-formatted string value for testGetValue which, though precise, requires parsing to be useful to users writing instrumentation tests.
If the goal is to follow exactly what the other bindings do, the ISO string returned should be exactly the same that will be recorded i.e. it is truncated to the time unit. This makes it even harder for parsing (in JS at least).
We can use glean-core's test_get_value_as_string
to make sure it works exactly like everywhere else. In hindsight that is what I should have done when first implementing this, oops.
However, that is for testGetValueAsString
. For testGetValue
we should return a date object. Not sure what that is for C++ in mozilla-central. For Javascript it is the Date object. That would match what Glean.js does.
Assignee | ||
Comment 2•3 years ago
|
||
Yeah, of the two (returning a truncated string, returning a better object) I prefer returning a nicer thing. Gaining compat with Glean.js is worth the effort on its own, and it means we can support a nice C++ API while we're at it.
Assignee | ||
Comment 3•3 years ago
|
||
Assignee | ||
Comment 4•3 years ago
|
||
Depends on D115375
Assignee | ||
Comment 5•3 years ago
|
||
Depends on D115376
Pushed by chutten@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/403d0030c4e9 Move test-only pref description to proper doc section r=janerik https://hg.mozilla.org/integration/autoland/rev/a16d49b60ee1 Break FOG's ffi mod into many mods r=janerik https://hg.mozilla.org/integration/autoland/rev/987000aaec55 Return Date types from FOG datetime testGetValue in C++ and JS r=brizental,janerik
Comment 7•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/403d0030c4e9
https://hg.mozilla.org/mozilla-central/rev/a16d49b60ee1
https://hg.mozilla.org/mozilla-central/rev/987000aaec55
Description
•