Closed Bug 1419996 Opened 2 years ago Closed 2 years ago

remove NEW_TABLE flag and legacy code for the onboarding telemetry

Categories

(Firefox :: Tours, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox60 --- fixed

People

(Reporter: gasolin, Assigned: mkohler)

References

Details

(Whiteboard: [onboarding-telemetry])

Attachments

(1 file)

We can remove NEW_TABLE flag and related legacy code when Bug 1413830, Bug 1412255 is landed.
Depends on: 1418167
Assignee: nobody → me
Status: NEW → ASSIGNED
Comment on attachment 8946404 [details]
Bug 1419996 - remove NEW_TABLE flag and legacy code for the onboarding telemetry

https://reviewboard.mozilla.org/r/216346/#review222314

Looks overrall good. One issue to fix.
p.s I have no L3 permission so not sure if this r+ is still able to land

::: browser/extensions/onboarding/OnboardingTelemetry.jsm
(Diff revision 1)
> -    if (!topic) {
> -      throw new Error(`ping-centre doesn't know ${event}, only knows ${Object.keys(OLD_EVENT_WHITELIST)}`);
> -    }
> -
> -    if (event === "onboarding-register-session") {
> -      this.registerNewTelemetrySession(data);

This `registerNewTelemetrySession` method should be removed as well
Attachment #8946404 - Flags: review?(fischer.json) → review+
(In reply to Fischer [:Fischer] from comment #2)
> Comment on attachment 8946404 [details]
> Bug 1419996 - remove NEW_TABLE flag and legacy code for the onboarding
> telemetry
> 
> https://reviewboard.mozilla.org/r/216346/#review222314
> 
> Looks overrall good. One issue to fix.
> p.s I have no L3 permission so not sure if this r+ is still able to land

No worries, I'll just set the checkin-needed keyword (will push to try in a second).

> ::: browser/extensions/onboarding/OnboardingTelemetry.jsm
> (Diff revision 1)
> > -    if (!topic) {
> > -      throw new Error(`ping-centre doesn't know ${event}, only knows ${Object.keys(OLD_EVENT_WHITELIST)}`);
> > -    }
> > -
> > -    if (event === "onboarding-register-session") {
> > -      this.registerNewTelemetrySession(data);
> 
> This `registerNewTelemetrySession` method should be removed as well

Good catch, removed!
Attachment #8946404 - Flags: review?(francois)
Attachment #8946404 - Flags: review?(francois)
No need for a data review if you're just _removing_ telemetry probes, but thanks for checking.
Kind of screwed up try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=456d8468819a08d6b0d1b6ae24afe5a9171eeae6&selectedJob=160445156

Setting the checkin-needed keyword anyway, feel free to not take it and let me run another one.
Keywords: checkin-needed
For landing this patch it has to be r+, in the upper left side there is just r?, could you please take a look?
Flags: needinfo?(francois)
Keywords: checkin-needed
The patch has already been r+, it looks ready to go.
Flags: needinfo?(francois)
Comment on attachment 8946404 [details]
Bug 1419996 - remove NEW_TABLE flag and legacy code for the onboarding telemetry

https://reviewboard.mozilla.org/r/216346/#review224278

datareview not necessary
Attachment #8946404 - Flags: review+
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/f390959ffb3c
remove NEW_TABLE flag and legacy code for the onboarding telemetry r=Fischer,francois
https://hg.mozilla.org/mozilla-central/rev/f390959ffb3c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
You need to log in before you can comment on or make changes to this bug.