Status

()

Core
General
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: (dormant account), Assigned: (dormant account))

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Clean up histogram API to be more coherent by passing histogram type in as a parameter. Also rename C++ implementation filename and add a namespace.
(Assignee)

Comment 1

6 years ago
Created attachment 533034 [details] [diff] [review]
cleanup
Assignee: nobody → tglek
Attachment #533034 - Flags: review?(mrbkap)
Blocks: 657297
How about https://bugzilla.mozilla.org/show_bug.cgi?id=649502#c11 ?
(In reply to comment #2)
> How about https://bugzilla.mozilla.org/show_bug.cgi?id=649502#c11 ?

Oh I didn't see this was (mostly) addressed in the patch. Why not the mozilla namespace?
(Assignee)

Comment 4

6 years ago
(In reply to comment #3)
> (In reply to comment #2)
> > How about https://bugzilla.mozilla.org/show_bug.cgi?id=649502#c11 ?
> 
> Oh I didn't see this was (mostly) addressed in the patch. Why not the
> mozilla namespace?

I don't see the benefit of it. The anonymous namespace allows better linking/optimization behavior. There is currently no need for the mozilla namespace, can be added later when there is something that uses or goes into it.
(Assignee)

Comment 5

6 years ago
Created attachment 533479 [details] [diff] [review]
cleanup

Minor update to previous patch:
* Removed toplevel static from Telemetry.cpp (made redundant by anonymous namespace)
* vesion -> version typo fix.
Attachment #533034 - Attachment is obsolete: true
Attachment #533034 - Flags: review?(mrbkap)
Attachment #533479 - Flags: review?(mrbkap)
Comment on attachment 533479 [details] [diff] [review]
cleanup

Review of attachment 533479 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #533479 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 7

6 years ago
http://hg.mozilla.org/mozilla-central/rev/8c4813583040
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.