1419996 - remove NEW_TABLE flag and legacy code for the onboarding telemetry

Status: RESOLVED FIXED

(Firefox :: Tours, enhancement, P3)

Description

[[e-mail]]

We can remove NEW_TABLE flag and related legacy code when Bug 1413830, Bug 1412255 is landed.

Comment 1

[Michael Kohler [:mkohler]]

Attachment: Bug 1419996 - remove NEW_TABLE flag and legacy code for the onboarding telemetry

Review commit: https://reviewboard.mozilla.org/r/216346/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216346/

Comment 2

[Fischer [:Fischer]]

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

Comment 3

[Michael Kohler [:mkohler]]

Comment on attachment 8946404 [details]
Bug 1419996 - remove NEW_TABLE flag and legacy code for the onboarding telemetry

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216346/diff/1-2/

Comment 4

[Michael Kohler [:mkohler]]

(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!

Comment 5

[François Marier [:francois]]

No need for a data review if you're just _removing_ telemetry probes, but thanks for checking.

Comment 6

[Michael Kohler [:mkohler]]

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.

Comment 7

[Cristina Coroiu [:ccoroiu]]

For landing this patch it has to be r+, in the upper left side there is just r?, could you please take a look?

Comment 8

[François Marier [:francois]]

The patch has already been r+, it looks ready to go.

Comment 9

[François Marier [: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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f390959ffb3c
remove NEW_TABLE flag and legacy code for the onboarding telemetry r=Fischer,francois

Comment 11

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/f390959ffb3c