Closed Bug 1234526 Opened 10 years ago Closed 10 years ago

Remove services/healthreport

Categories

(Firefox Health Report Graveyard :: Client: Desktop, defect, P1)

defect

Tracking

(firefox46 fixed)

RESOLVED FIXED
Firefox 46
Tracking Status
firefox46 --- fixed

People

(Reporter: Dexter, Assigned: Dexter)

References

Details

(Whiteboard: [measurement:client])

Attachments

(3 files, 1 obsolete file)

This bug is about removing services/metrics & services/healthreport from Firefox codebase. We should also take care of removing: - Orphaned manifest entries: https://dxr.mozilla.org/mozilla-central/search?q=%22HealthReport.jsm%22+-path%3Aobj&redirect=false&case=false - Uses of Metrics.jsm throughout the codebase
No longer depends on: 1234522
Points: --- → 3
Changed this bug to only remove services/healthreport to ease reviewing.
Assignee: nobody → alessio.placitelli
Status: NEW → ASSIGNED
Summary: Remove services/metrics & services/healthreport → Remove services/healthreport
Blocks: 1235345
Depends on: 1234522
This bug should also investigate and eventually remove all the uses of the FHR prefs (listed in [0]), except the following: - datareporting.healthreport.infoURL - Used to build the preferences panel - datareporting.healthreport.uploadEnabled - Used to control ping upload in Telemetry - datareporting.healthreport.about.reportUrl - Used for the about:healthreport content URL / former v2 URL - datareporting.healthreport.about.reportUrlUnified - Used for the about:healthreport content URL / UT URL [0] - https://dxr.mozilla.org/mozilla-central/rev/388bdc46ba51ee31da8b8abe977e0ca38d117434/services/healthreport/healthreport-prefs.js
Priority: P2 → P1
(In reply to Alessio Placitelli [:Dexter] from comment #2) > This bug should also investigate and eventually remove all the uses of the > FHR prefs (listed in [0]), except the following: > > - datareporting.healthreport.infoURL - Used to build the preferences panel > - datareporting.healthreport.uploadEnabled - Used to control ping upload in > Telemetry > - datareporting.healthreport.about.reportUrl - Used for the > about:healthreport content URL / former v2 URL > - datareporting.healthreport.about.reportUrlUnified - Used for the > about:healthreport content URL / UT URL This was planned to be used, but we decided to go a different route (see bug 1227579). We should remove that and any uses in code (potentially in a separate bug).
Attachment #8704160 - Flags: review?(gfritzsche)
Attachment #8704162 - Flags: review?(gfritzsche)
Comment on attachment 8704160 [details] [diff] [review] part 1 - Remove services/healthreport Review of attachment 8704160 [details] [diff] [review]: ----------------------------------------------------------------- This is not removing test_healthreporter.js. ::: mobile/android/installer/package-manifest.in @@ -440,5 @@ > > -#ifdef MOZ_SERVICES_HEALTHREPORT > -@BINPATH@/components/HealthReportComponents.manifest > -@BINPATH@/components/HealthReportService.js > -#endif Already removed: https://hg.mozilla.org/mozilla-central/rev/de71199f4e00 ::: modules/libpref/greprefs.js @@ +6,5 @@ > #ifdef MOZ_SERVICES_HEALTHREPORT > #if MOZ_WIDGET_TOOLKIT == android > #include ../../mobile/android/chrome/content/healthreport-prefs.js > #else > +#include ../../toolkit/components/telemetry/healthreport-prefs.js Lets file something about consolidating those prefs into one file, telemetry-prefs.js ::: services/healthreport/docs/dataformat.rst @@ -1,1 @@ > -.. _healthreport_dataformat: People might still have to look at this for a bit. Lets move this to the Telemetry doc dir and index (so we can kill the FHR dirs) and mark it with a clear disclaimer on FHR being obsolete.
Attachment #8704160 - Flags: review?(gfritzsche) → review+
Comment on attachment 8704162 [details] [diff] [review] part 2 - Remove unused prefs Review of attachment 8704162 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/telemetry/healthreport-prefs.js @@ +11,2 @@ > pref("datareporting.healthreport.about.reportUrl", "https://fhr.cdn.mozilla.net/%LOCALE%/v4/"); > pref("datareporting.healthreport.about.reportUrlUnified", "https://fhr.cdn.mozilla.net/%LOCALE%/v4/"); Didn't we want to kill .reportUrlUnified somewhere? I'd be happy to have it in a separate bug as long as it happens.
Attachment #8704162 - Flags: review?(gfritzsche) → review+
Attachment #8704160 - Attachment is obsolete: true
Attachment #8704693 - Flags: review+
(In reply to Georg Fritzsche [:gfritzsche] [away until Jan 3] from comment #6) > Comment on attachment 8704160 [details] [diff] [review] > part 1 - Remove services/healthreport > > Review of attachment 8704160 [details] [diff] [review]: > ----------------------------------------------------------------- > > This is not removing test_healthreporter.js. This is being removed by bug 1234522. > ::: mobile/android/installer/package-manifest.in > @@ -440,5 @@ > > > > -#ifdef MOZ_SERVICES_HEALTHREPORT > > -@BINPATH@/components/HealthReportComponents.manifest > > -@BINPATH@/components/HealthReportService.js > > -#endif > > Already removed: https://hg.mozilla.org/mozilla-central/rev/de71199f4e00 Ah, good! > ::: modules/libpref/greprefs.js > @@ +6,5 @@ > > #ifdef MOZ_SERVICES_HEALTHREPORT > > #if MOZ_WIDGET_TOOLKIT == android > > #include ../../mobile/android/chrome/content/healthreport-prefs.js > > #else > > +#include ../../toolkit/components/telemetry/healthreport-prefs.js > > Lets file something about consolidating those prefs into one file, > telemetry-prefs.js Filed bug 1237276. > ::: services/healthreport/docs/dataformat.rst > @@ -1,1 @@ > > -.. _healthreport_dataformat: > > People might still have to look at this for a bit. > Lets move this to the Telemetry doc dir and index (so we can kill the FHR > dirs) and mark it with a clear disclaimer on FHR being obsolete. Good point.
(In reply to Georg Fritzsche [:gfritzsche] [away until Jan 3] from comment #7) > Comment on attachment 8704162 [details] [diff] [review] > part 2 - Remove unused prefs > > Review of attachment 8704162 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: toolkit/components/telemetry/healthreport-prefs.js > @@ +11,2 @@ > > pref("datareporting.healthreport.about.reportUrl", "https://fhr.cdn.mozilla.net/%LOCALE%/v4/"); > > pref("datareporting.healthreport.about.reportUrlUnified", "https://fhr.cdn.mozilla.net/%LOCALE%/v4/"); > > Didn't we want to kill .reportUrlUnified somewhere? > I'd be happy to have it in a separate bug as long as it happens. That's being done as part of bug 1236580.
Attachment #8705077 - Flags: review?(gfritzsche) → review+
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: