Last Comment Bug 733510 - Transition boolean a11y telemetry to 'flag' telemetry as appropriate
: Transition boolean a11y telemetry to 'flag' telemetry as appropriate
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Trevor Saunders (:tbsaunde)
:
Mentors:
Depends on: 723846
Blocks: telemetrya11y
  Show dependency treegraph
 
Reported: 2012-03-06 12:52 PST by David Bolter [:davidb]
Modified: 2012-04-05 11:24 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.94 KB, patch)
2012-03-27 11:23 PDT, Trevor Saunders (:tbsaunde)
dbolter: review+
Details | Diff | Splinter Review
patch (3.41 KB, patch)
2012-03-27 13:45 PDT, Trevor Saunders (:tbsaunde)
nfroyd: review+
Details | Diff | Splinter Review
patch 2 (3.45 KB, patch)
2012-04-04 17:47 PDT, Trevor Saunders (:tbsaunde)
no flags Details | Diff | Splinter Review

Description David Bolter [:davidb] 2012-03-06 12:52:14 PST
Placeholder so we don't forget.
Comment 1 David Bolter [:davidb] 2012-03-23 08:27:26 PDT
Thanks for taking this one Trevor.
Comment 2 Trevor Saunders (:tbsaunde) 2012-03-27 11:23:42 PDT
Created attachment 609803 [details] [diff] [review]
patch
Comment 3 David Bolter [:davidb] 2012-03-27 11:35:23 PDT
Comment on attachment 609803 [details] [diff] [review]
patch

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

r=me thanks, but please change the appropriate accumulate calls to take [true] instead of [1] (in Statistics.h).

Then ask a toolkit peer to review ;)
Comment 4 Trevor Saunders (:tbsaunde) 2012-03-27 13:45:28 PDT
Created attachment 609870 [details] [diff] [review]
patch
Comment 5 Nathan Froyd [:froydnj] 2012-03-28 09:29:47 PDT
Comment on attachment 609870 [details] [diff] [review]
patch

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

r=me with the changes below.

::: toolkit/components/telemetry/TelemetryHistograms.h
@@ +59,5 @@
>  
>  /**
>   * a11y telemetry
>   */
> +HISTOGRAM_FLAG(A11Y_INSTANTIATED, "has accessibility support been instantiated")

Please rename the histograms (perhaps FOO -> FOO_FLAG) so that we can clearly distinguish them in the Telemetry dashboard.
Comment 6 (dormant account) 2012-04-02 15:03:16 PDT
Comment on attachment 609870 [details] [diff] [review]
patch

This is going to mess up old data. We should transition changes in what histograms are measuring to using new names(or atleast notify metrics to delete old histograms if they are not useful).
Comment 7 David Bolter [:davidb] 2012-04-04 14:12:00 PDT
I'm leaning toward landing the patch, deleting the old data and moving on. We can have a last look at the existing data before doing this of course. Also we'd want to uplift this patch to aurora so that we can start getting the new data asap.
Comment 8 Nathan Froyd [:froydnj] 2012-04-04 14:29:18 PDT
(In reply to David Bolter [:davidb] from comment #7)
> I'm leaning toward landing the patch, deleting the old data and moving on.
> We can have a last look at the existing data before doing this of course.
> Also we'd want to uplift this patch to aurora so that we can start getting
> the new data asap.

Do note that you will continue--or at least have the potential to continue--getting data from beta and any versions that released with the existing histogram name until everybody upgrades, which may be a very long time for old and new data to co-exist.

I guess you could use the telemetry dashboard to filter out anything earlier than mozilla14-ish.
Comment 9 Trevor Saunders (:tbsaunde) 2012-04-04 14:49:41 PDT
(In reply to David Bolter [:davidb] from comment #7)
> I'm leaning toward landing the patch, deleting the old data and moving on.
> We can have a last look at the existing data before doing this of course.
> Also we'd want to uplift this patch to aurora so that we can start getting
> the new data asap.

I don't really care either way, but fixing comment 5 should be all of about 3 minutes of mechanical changes to two files, and another five minutes of kicking off a build and uploading a patch.
Comment 10 David Bolter [:davidb] 2012-04-04 15:03:44 PDT
(In reply to Trevor Saunders (:tbsaunde) from comment #9)
> (In reply to David Bolter [:davidb] from comment #7)
> > I'm leaning toward landing the patch, deleting the old data and moving on.
> > We can have a last look at the existing data before doing this of course.
> > Also we'd want to uplift this patch to aurora so that we can start getting
> > the new data asap.
> 
> I don't really care either way, but fixing comment 5 should be all of about
> 3 minutes of mechanical changes to two files, and another five minutes of
> kicking off a build and uploading a patch.

Yeah ok. Go for it! :)
Comment 11 Trevor Saunders (:tbsaunde) 2012-04-04 17:47:46 PDT
Created attachment 612413 [details] [diff] [review]
patch 2
Comment 12 Trevor Saunders (:tbsaunde) 2012-04-04 18:06:59 PDT
Comment on attachment 612413 [details] [diff] [review]
patch 2

err, seems I can't read bugmail :(
Comment 13 Trevor Saunders (:tbsaunde) 2012-04-04 21:31:56 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/e529dbc6eddb
Comment 14 alexander :surkov 2012-04-04 22:28:41 PDT
(In reply to David Bolter [:davidb] from comment #7)
> I'm leaning toward landing the patch, deleting the old data and moving on.
> We can have a last look at the existing data before doing this of course.
> Also we'd want to uplift this patch to aurora so that we can start getting
> the new data asap.

I would go the same way. Old data wasn't really useful. New names aren't super nice.
Comment 15 David Bolter [:davidb] 2012-04-05 07:41:46 PDT
Discussion on IRC brought us all on the same page. Let's keep this as is.

Note You need to log in before you can comment on or make changes to this bug.