Closed Bug 1382196 Opened 2 years ago Closed 2 years ago

Use the prefs defined in TelemetryUtils.jsm in TelemetryReportingPolicy.jsm

Categories

(Toolkit :: Telemetry, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: alejandro, Assigned: alejandro, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [measurement:client])

Attachments

(1 file)

We should change Telemetry code to use the preferences defined in the TelemetryUtils.jsm file rather than redefining them in TelemetryReportingPolicy.jsm.
Whiteboard: [measurement:client]
Assignee: nobody → alexrs95
Comment on attachment 8890354 [details] [diff] [review]
Use the prefs defined in TelemetryUtils.jsm in TelemetryReportingPolicy.jsm

Review of attachment 8890354 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good with the nit below addressed.

::: toolkit/components/telemetry/TelemetryUtils.jsm
@@ +49,5 @@
>      CurrentPolicyVersion: "datareporting.policy.currentPolicyVersion",
>      DataSubmissionEnabled: "datareporting.policy.dataSubmissionEnabled",
>      FhrUploadEnabled: "datareporting.healthreport.uploadEnabled",
>      MinimumPolicyVersion: "datareporting.policy.minimumPolicyVersion",
> +    FirstRunURL: "datareporting.policy.firstRunURL",

Can you move this one near FirstRun?
Attachment #8890354 - Flags: review?(alessio.placitelli) → feedback+
(In reply to Alessio Placitelli [:Dexter] from comment #2)
> Comment on attachment 8890354 [details] [diff] [review]
> Use the prefs defined in TelemetryUtils.jsm in TelemetryReportingPolicy.jsm
> 
> Review of attachment 8890354 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks good with the nit below addressed.
> 
> ::: toolkit/components/telemetry/TelemetryUtils.jsm
> @@ +49,5 @@
> >      CurrentPolicyVersion: "datareporting.policy.currentPolicyVersion",
> >      DataSubmissionEnabled: "datareporting.policy.dataSubmissionEnabled",
> >      FhrUploadEnabled: "datareporting.healthreport.uploadEnabled",
> >      MinimumPolicyVersion: "datareporting.policy.minimumPolicyVersion",
> > +    FirstRunURL: "datareporting.policy.firstRunURL",
> 
> Can you move this one near FirstRun?

Now the prefs are ordered by branch name. "toolkit.telemetry.*" first, then "datareporting.policy.*". FirstRun belongs to the first group, and FirstRunURL to the second one. Should I break this order to move FirstRunURL near FirstRun?
(In reply to Alejandro Rodriguez Salamanca from comment #3)
> (In reply to Alessio Placitelli [:Dexter] from comment #2)
> > Comment on attachment 8890354 [details] [diff] [review]
> > Use the prefs defined in TelemetryUtils.jsm in TelemetryReportingPolicy.jsm
> > 
> > Review of attachment 8890354 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > This looks good with the nit below addressed.
> > 
> > ::: toolkit/components/telemetry/TelemetryUtils.jsm
> > @@ +49,5 @@
> > >      CurrentPolicyVersion: "datareporting.policy.currentPolicyVersion",
> > >      DataSubmissionEnabled: "datareporting.policy.dataSubmissionEnabled",
> > >      FhrUploadEnabled: "datareporting.healthreport.uploadEnabled",
> > >      MinimumPolicyVersion: "datareporting.policy.minimumPolicyVersion",
> > > +    FirstRunURL: "datareporting.policy.firstRunURL",
> > 
> > Can you move this one near FirstRun?
> 
> Now the prefs are ordered by branch name. "toolkit.telemetry.*" first, then
> "datareporting.policy.*". FirstRun belongs to the first group, and
> FirstRunURL to the second one. Should I break this order to move FirstRunURL
> near FirstRun?

Nope, good point.
https://hg.mozilla.org/mozilla-central/rev/a86a44628486
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.