733510 - Transition boolean a11y telemetry to 'flag' telemetry as appropriate

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[David Bolter [:davidb] (NeedInfo me for attention)]

Placeholder so we don't forget.

Comment 1

[David Bolter [:davidb] (NeedInfo me for attention)]

Thanks for taking this one Trevor.

Comment 2

[Trevor Saunders (:tbsaunde)]

Attachment: patch

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

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)]

Attachment: patch

Comment 5

[Nathan Froyd [:froydnj]]

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)]

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] (NeedInfo me for attention)]

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]]

(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)]

(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] (NeedInfo me for attention)]

(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)]

Attachment: patch 2

Comment 12

[Trevor Saunders (:tbsaunde)]

Comment on attachment 612413 [details] [diff] [review]
patch 2

err, seems I can't read bugmail :(

Comment 13

[Trevor Saunders (:tbsaunde)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e529dbc6eddb

Comment 14

[alexander :surkov (:asurkov)]

(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] (NeedInfo me for attention)]

Discussion on IRC brought us all on the same page. Let's keep this as is.

Comment 16

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/e529dbc6eddb