657709 - Cleanup histogram API

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[(dormant account)]

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.

Comment 1

[(dormant account)]

Attachment: cleanup

Comment 2

[Mike Hommey [:glandium]]

How about https://bugzilla.mozilla.org/show_bug.cgi?id=649502#c11 ?

Comment 3

[Mike Hommey [:glandium]]

(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?

Comment 4

[(dormant account)]

(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.

Comment 5

[(dormant account)]

Attachment: cleanup

Minor update to previous patch:
* Removed toplevel static from Telemetry.cpp (made redundant by anonymous namespace)
* vesion -> version typo fix.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 533479 [details] [diff] [review]
cleanup

Review of attachment 533479 [details] [diff] [review]:
-----------------------------------------------------------------

Comment 7

[(dormant account)]

http://hg.mozilla.org/mozilla-central/rev/8c4813583040