Closed Bug 657709 Opened 9 years ago Closed 9 years ago

Cleanup histogram API

Categories

(Core :: General, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: taras.mozilla, Assigned: taras.mozilla)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Attached patch cleanup (obsolete) — Splinter Review
Assignee: nobody → tglek
Attachment #533034 - Flags: review?(mrbkap)
(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?
(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.
Attached patch cleanupSplinter Review
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+
http://hg.mozilla.org/mozilla-central/rev/8c4813583040
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.