Closed
Bug 723181
Opened 12 years ago
Closed 12 years ago
add some comments to TelemetryTimestamps
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 13
People
(Reporter: Gavin, Assigned: Gavin)
Details
Attachments
(1 file)
2.35 KB,
patch
|
mak
:
review+
|
Details | Diff | Splinter Review |
Marco rightfully pointed out on IRC that it's not obvious what this module is for, or how it should be used. Let's add some comments!
Assignee | ||
Comment 1•12 years ago
|
||
Also made a slight tweak to the code, making "timeStamps" private to the module rather than a property on the exposed object.
Assignee | ||
Updated•12 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Comment 2•12 years ago
|
||
Comment on attachment 593508 [details] [diff] [review] patch Review of attachment 593508 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/modules/TelemetryTimestamps.jsm @@ +17,1 @@ > let TelemetryTimestamps = { nit: newline before and after timestamps var definition may help readability. @@ +22,5 @@ > + * "name" must be a unique, generally "camelCase" descriptor of what the > + * timestamp represents. e.g.: > + * sessionRestoreRestoring, delayedStartupStarted > + * "value" is a timeStamp in milliseconds since the epoch. If omitted, > + * defaults to Date.now(). may you use javadoc @param style here? @@ +42,5 @@ > + > + /** > + * Get a JS object containing all of the timeStamps as properties (can be > + * easily serialized to JSON). Used by TelemetryPing to retrieve the data > + * to attach to the telemetry submission. may get a @return javadoc part. @@ +48,2 @@ > get: function TT_get() { > + return JSON.parse(JSON.stringify(timeStamps)); may you please add a brief comment on why we actually have to stringify and parse the object? Is this for normalizing names or such?
Attachment #593508 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 3•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/62d547447987
Flags: in-testsuite-
Target Milestone: --- → Firefox 13
Comment 4•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/62d547447987
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•