Transition boolean a11y telemetry to 'flag' telemetry as appropriate

RESOLVED FIXED in mozilla14

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: davidb, Assigned: tbsaunde)

Tracking

unspecified
mozilla14
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Placeholder so we don't forget.
(Reporter)

Comment 1

5 years ago
Thanks for taking this one Trevor.
Assignee: nobody → trev.saunders
(Assignee)

Comment 2

5 years ago
Created attachment 609803 [details] [diff] [review]
patch
Attachment #609803 - Flags: review?(dbolter)
(Reporter)

Comment 3

5 years ago
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 ;)
Attachment #609803 - Flags: review?(dbolter) → review+
(Assignee)

Comment 4

5 years ago
Created attachment 609870 [details] [diff] [review]
patch
Attachment #609870 - Flags: review?(taras.mozilla)
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.
Attachment #609870 - Flags: review?(taras.mozilla) → review+

Comment 6

5 years ago
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).
(Reporter)

Updated

5 years ago
Blocks: 648121
(Reporter)

Comment 7

5 years ago
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.
(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.
(Assignee)

Comment 9

5 years ago
(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.
(Reporter)

Comment 10

5 years ago
(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! :)
(Assignee)

Comment 11

5 years ago
Created attachment 612413 [details] [diff] [review]
patch 2
Attachment #609803 - Attachment is obsolete: true
Attachment #609870 - Attachment is obsolete: true
Attachment #612413 - Flags: review?(nfroyd)
(Assignee)

Comment 12

5 years ago
Comment on attachment 612413 [details] [diff] [review]
patch 2

err, seems I can't read bugmail :(
Attachment #612413 - Flags: review?(nfroyd)
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e529dbc6eddb
Target Milestone: --- → mozilla14

Comment 14

5 years ago
(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.
Target Milestone: mozilla14 → ---
(Reporter)

Comment 15

5 years ago
Discussion on IRC brought us all on the same page. Let's keep this as is.
Target Milestone: --- → mozilla14
http://hg.mozilla.org/mozilla-central/rev/e529dbc6eddb
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.