Closed
Bug 1234526
Opened 8 years ago
Closed 8 years ago
Remove services/healthreport
Categories
(Firefox Health Report Graveyard :: Client: Desktop, defect, P1)
Firefox Health Report Graveyard
Client: Desktop
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)
16.79 KB,
patch
|
gfritzsche
:
review+
|
Details | Diff | Splinter Review |
179.94 KB,
patch
|
Dexter
:
review+
|
Details | Diff | Splinter Review |
1.16 KB,
patch
|
gfritzsche
:
review+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Updated•8 years ago
|
Points: --- → 3
Assignee | ||
Comment 1•8 years ago
|
||
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
Assignee | ||
Comment 2•8 years ago
|
||
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
Assignee | ||
Updated•8 years ago
|
Priority: P2 → P1
Comment 3•8 years ago
|
||
(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).
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8704160 -
Flags: review?(gfritzsche)
Assignee | ||
Comment 5•8 years ago
|
||
Attachment #8704162 -
Flags: review?(gfritzsche)
Comment 6•8 years ago
|
||
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 7•8 years ago
|
||
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+
Assignee | ||
Comment 8•8 years ago
|
||
Attachment #8704160 -
Attachment is obsolete: true
Attachment #8704693 -
Flags: review+
Assignee | ||
Comment 9•8 years ago
|
||
(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.
Assignee | ||
Comment 10•8 years ago
|
||
(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.
Assignee | ||
Comment 11•8 years ago
|
||
Attachment #8705077 -
Flags: review?(gfritzsche)
Updated•8 years ago
|
Attachment #8705077 -
Flags: review?(gfritzsche) → review+
Assignee | ||
Comment 12•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e0b62d85cb8
Assignee | ||
Comment 13•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/096d46bdaf86112bc1bf350fec18d5d4063ab0c2 Bug 1234526 - Remove services/healthreport. r=gfritzsche https://hg.mozilla.org/integration/fx-team/rev/3124025d114712f6717e6cf6a4ba930f0ab02ac3 Bug 1234526 - Remove unused healthreporter prefs. r=gfritzsche https://hg.mozilla.org/integration/fx-team/rev/c7689b72d3fa71043aa8601134b7bdc4581deb86 Bug 1234526 - Don't track healthreport.sqlite statements from Telemetry. r=gfritzsche
Comment 14•8 years ago
|
||
Backed out the whole push in https://hg.mozilla.org/integration/fx-team/rev/19a2342819e4 - see bug 1234522 comment 23
Assignee | ||
Comment 15•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/839ed1dfce6477dc20a77b7f1465188af1068c04 Bug 1234526 - Remove services/healthreport. r=gfritzsche https://hg.mozilla.org/integration/fx-team/rev/a90decf2cd6a344110d74c5473f02255cb0d06ac Bug 1234526 - Remove unused healthreporter prefs. r=gfritzsche https://hg.mozilla.org/integration/fx-team/rev/86e069b34fb4690ac6f5e959ef7d259014307641 Bug 1234526 - Don't track healthreport.sqlite statements from Telemetry. r=gfritzsche
Comment 17•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/839ed1dfce64 https://hg.mozilla.org/mozilla-central/rev/a90decf2cd6a https://hg.mozilla.org/mozilla-central/rev/86e069b34fb4
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Updated•6 years ago
|
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•