Closed Bug 733510 Opened 9 years ago Closed 9 years ago

Transition boolean a11y telemetry to 'flag' telemetry as appropriate

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla14

People

(Reporter: davidb, Assigned: tbsaunde)

References

Details

Attachments

(1 file, 2 obsolete files)

Placeholder so we don't forget.
Thanks for taking this one Trevor.
Assignee: nobody → trev.saunders
Attached patch patch (obsolete) — Splinter Review
Attachment #609803 - Flags: review?(dbolter)
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+
Attached patch patch (obsolete) — Splinter Review
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 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).
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.
(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.
(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! :)
Attached patch patch 2Splinter Review
Attachment #609803 - Attachment is obsolete: true
Attachment #609870 - Attachment is obsolete: true
Attachment #612413 - Flags: review?(nfroyd)
Comment on attachment 612413 [details] [diff] [review]
patch 2

err, seems I can't read bugmail :(
Attachment #612413 - Flags: review?(nfroyd)
(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 → ---
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
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.