Closed
Bug 1234526
Opened 10 years ago
Closed 10 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•10 years ago
|
Points: --- → 3
Assignee | ||
Comment 1•10 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•10 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•10 years ago
|
Priority: P2 → P1
Comment 3•10 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•10 years ago
|
||
Attachment #8704160 -
Flags: review?(gfritzsche)
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8704162 -
Flags: review?(gfritzsche)
Comment 6•10 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•10 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•10 years ago
|
||
Attachment #8704160 -
Attachment is obsolete: true
Attachment #8704693 -
Flags: review+
Assignee | ||
Comment 9•10 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•10 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•10 years ago
|
||
Attachment #8705077 -
Flags: review?(gfritzsche)
Updated•10 years ago
|
Attachment #8705077 -
Flags: review?(gfritzsche) → review+
Assignee | ||
Comment 12•10 years ago
|
||
Assignee | ||
Comment 13•10 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•10 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•10 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•10 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: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Updated•7 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
•