Remove RECORD_PREF_NOTIFY_ONLY from TelemetryEnvironment

RESOLVED FIXED in Firefox 43



4 years ago
4 years ago


(Reporter: gfritzsche, Assigned: robertthyberg)


Dependency tree / graph

Firefox Tracking Flags

(firefox42 wontfix, firefox43 fixed)


(Whiteboard: [lang=js] [good first bug])


(1 attachment, 1 obsolete attachment)

We don't actually use the RECORD_PREF_NOTIFY_ONLY behavior for any pref, so we should remove it and any logic that handles it:
Are you sure we don't need it? There are a bunch of things in the settings block which are based on prefs and can change mid-run.
Yes, we have 3 different behaviors:
  RECORD_PREF_STATE: 1, // Don't record the preference value
  RECORD_PREF_VALUE: 2, // We only record user-set prefs.
  RECORD_PREF_NOTIFY_ONLY: 3, // Record nothing, just notify of changes.

Everything we do is covered by the first two, the third is unused and of dubious value.
can I take this bug?
Sure, assigned it to you :)
Assignee: nobody → robertthyberg
should i remove it from other files as well? or just from TelemetryEnvironmret
Per the code search in comment 0 this isn't used anywhere else, so removing from TelemetryEnvironment should be all.
Attachment #8650145 - Attachment is patch: true
Attachment #8650145 - Flags: review?( → review?(gfritzsche)
Comment on attachment 8650145 [details] [diff] [review]
removed references to RECORD_PREF_NOTIFY and adjusted the logic relating to it

Review of attachment 8650145 [details] [diff] [review]:

This looks good to me with the below fixed.

Note that we have to finish bug 1192811 first before we can land this bug.

::: toolkit/components/telemetry/TelemetryEnvironment.jsm
@@ -768,3 @@
>          continue;
>        }
> -

Don't remove the empty line here, there is no need to.
Attachment #8650145 - Flags: review?(gfritzsche) → review+
undid the line removal
Flags: needinfo?(gfritzsche)
Pushed to try together with bug 1192811:
Flags: needinfo?(gfritzsche)
Duplicate of this bug: 1137178
Attachment #8650145 - Attachment is obsolete: true
Attachment #8650690 - Flags: review+
The try run from comment 10 is all green, so setting checkin-needed.
Keywords: checkin-needed
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.