Closed
Bug 1270641
Opened 9 years ago
Closed 8 years ago
Choose winner of expanded-firstrun and uplift through 47
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox47 wontfix, firefox48 verified, firefox49 unaffected, fennec48+)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox47 | --- | wontfix |
firefox48 | --- | verified |
firefox49 | --- | unaffected |
fennec | 48+ | --- |
People
(Reporter: liuche, Assigned: liuche)
References
Details
Attachments
(1 file, 1 obsolete file)
19.95 KB,
patch
|
liuche
:
review+
|
Details | Diff | Splinter Review |
We need to pick a first run (A, B, C) as the winner and uplift that through Beta.
Assignee | ||
Comment 1•9 years ago
|
||
Barbara, let me know when we'll have an idea about this one, or when we'll have enough data from beta to make this decision.
Flags: needinfo?(bbermes)
Updated•9 years ago
|
tracking-fennec: --- → 47+
Assignee | ||
Comment 2•8 years ago
|
||
Alex, do you know which version won? Was it the one without the Data Saver panel?
Flags: needinfo?(adavis)
Comment 3•8 years ago
|
||
Are you referring to this onboarding test? https://mana.mozilla.org/wiki/pages/viewpage.action?pageId=57508096 Barbara logged her results but didn't formulate a conclusion in the document so I am unsure what she decided to do moving forward. Version B demonstrated a slight improvement but I don't know if she felt it was worth rolling out.
Flags: needinfo?(adavis)
Assignee | ||
Comment 4•8 years ago
|
||
Barbara, what do you think? Let me know if you have a timeline for when we can pick one of these! If it's long, we might want to just land bug 1268647 with Version B, and then decide later (or Version C, either is fine with me).
Comment 5•8 years ago
|
||
Up until today, I thought B would be a slight winner, however looking at the latest results, C is showing some success now too: https://sql.telemetry.mozilla.org/dashboard/experiment-onboarding-with-multiple-slides. When do you need this decision by? I'm also fine with going with B for now if that's easier, but wanted to bring this up. ADavis, what are you saying?
Flags: needinfo?(bbermes) → needinfo?(adavis)
Assignee | ||
Comment 6•8 years ago
|
||
Okay, thanks! In that case, if we won't know for a while, I'll just go with C - it'll be easier to remove code rather than add it back in. I'll just land things today, and start working on the uplift patch for getting en_US only in 48 (bug 1268647).
Assignee | ||
Comment 7•8 years ago
|
||
To be clear, I'm not landing anything for this bug, but am rather talking about bug 1268647.
Comment 8•8 years ago
|
||
In the dashboard I made, B & C were tied as winners for best retention. C works for me.
Updated•8 years ago
|
Flags: needinfo?(adavis)
Updated•8 years ago
|
Assignee: nobody → liuche
Assignee | ||
Comment 9•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/55546/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55546/
Attachment #8757028 -
Flags: review?(ahunt)
Updated•8 years ago
|
Attachment #8757028 -
Flags: review?(ahunt) → review+
Comment 10•8 years ago
|
||
Comment on attachment 8757028 [details] MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt https://reviewboard.mozilla.org/r/55546/#review52312 ::: mobile/android/base/java/org/mozilla/gecko/util/Experiments.java:91 (Diff revision 1) > - if (SwitchBoard.isInBucket(context, 0, 33)) { > - return Experiments.ONBOARDING2_A.equals(experiment); > - } else if (SwitchBoard.isInBucket(context, 33, 66)) { > - return Experiments.ONBOARDING2_B.equals(experiment); > - } else if (SwitchBoard.isInBucket(context, 66, 100)) { > - return Experiments.ONBOARDING2_C.equals(experiment); > + return Experiments.ONBOARDING2_C.equals(experiment); Do we need to keep this at all? It looks like we removed the check in FirstrunPagerConfig.getDefault.
Assignee | ||
Comment 11•8 years ago
|
||
(In reply to Andrzej Hunt :ahunt from comment #10) > Comment on attachment 8757028 [details] > MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt > > https://reviewboard.mozilla.org/r/55546/#review52312 > > ::: mobile/android/base/java/org/mozilla/gecko/util/Experiments.java:91 > (Diff revision 1) > > - if (SwitchBoard.isInBucket(context, 0, 33)) { > > - return Experiments.ONBOARDING2_A.equals(experiment); > > - } else if (SwitchBoard.isInBucket(context, 33, 66)) { > > - return Experiments.ONBOARDING2_B.equals(experiment); > > - } else if (SwitchBoard.isInBucket(context, 66, 100)) { > > - return Experiments.ONBOARDING2_C.equals(experiment); > > + return Experiments.ONBOARDING2_C.equals(experiment); > > Do we need to keep this at all? It looks like we removed the check in > FirstrunPagerConfig.getDefault. It's still being used by the Kinto downloadable content (incorrectly, I think) but I commented on that to see if it was intended to be a live Switchboard query, or perhaps it's meant to be added to this call. Aurora try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f73acc49f46
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8757028 [details] MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt Approval Request Comment [Feature/regressing bug #]: Turn off experiment [User impact if declined]: Users will still be in an experiment [Describe test coverage new/current, TreeHerder]: local, try runs [Risks and why]: Low, removing code [String/UUID change made/needed]: removal of unused strings, no need to re-localize
Attachment #8757028 -
Flags: approval-mozilla-beta?
Attachment #8757028 -
Flags: approval-mozilla-aurora?
Updated•8 years ago
|
status-firefox47:
--- → affected
status-firefox48:
--- → affected
Comment on attachment 8757028 [details] MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt I spoke to Margaret and mentioned the fact that RC week is only for release blocking issues. Keeping that bar in mind, we may not have enough time to stop this experiment. Let's uplift only to Aurora48.
Attachment #8757028 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
Updated•8 years ago
|
tracking-fennec: 47+ → 48+
Comment 14•8 years ago
|
||
any reason why this didn't land in nightly?
Comment 15•8 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #14) > any reason why this didn't land in nightly? The first run flow is different on 49+ (bug 1268641), so this bug didn't affect Nightly. Why has this still not landed on Aurora? At this point this will need to be uplifted to Beta now.
Assignee | ||
Comment 16•8 years ago
|
||
Comment on attachment 8757028 [details] MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt Trying to flag this for beta, because this missed aurora merge, but it looks like I can't clear the b- flag.
Flags: needinfo?(rkothari)
Attachment #8757028 -
Flags: approval-mozilla-aurora?
Comment on attachment 8757028 [details] MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt This is approved for uplift to Beta48.
Flags: needinfo?(rkothari)
Attachment #8757028 -
Flags: approval-mozilla-beta- → approval-mozilla-beta+
Hi Wes, just fyi, this needs to land on Beta48. Thanks!
Flags: needinfo?(wkocher)
Comment 19•8 years ago
|
||
Is it possible to have a patch that doesn't touch localization files for string frozen branches (aurora, beta)? Removing strings from localization files is going to create unnecessary noise in tools and dashboards, so having a patch without changes to android_strings.dtd would be ideal. The patch removing strings can then ride the trains. To be honest it's also absolutely unclear why this never landed in nightly and it's jumping directly to beta.
Flags: needinfo?(liuche)
Comment 20•8 years ago
|
||
i guess we need to wait till comment#19 is fixed before we can uplift
Flags: needinfo?(wkocher)
Assignee | ||
Comment 21•8 years ago
|
||
Carrying over review flag, removed dtd changes. > To be honest it's also absolutely unclear why this never landed in nightly > and it's jumping directly to beta. Margaret addressed this in Comment #15, and the tracking flags also show that neither 49 nor 50 are affected. Nightly (at the time of the a? request) had a completely different flow, and this didn't get uplifted so now we're landing it on beta.
Attachment #8757028 -
Attachment is obsolete: true
Flags: needinfo?(liuche)
Attachment #8761860 -
Flags: review+
Assignee | ||
Comment 22•8 years ago
|
||
Ready to land now, I removed the string changes. Beta try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eff519e8b258
Flags: needinfo?(cbook)
Comment 23•8 years ago
|
||
landed! -> https://hg.mozilla.org/releases/mozilla-beta/rev/1510116bd5e5
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(cbook)
Resolution: --- → FIXED
Comment 24•8 years ago
|
||
Verified as fixed on latest Beta build, 48.0b2. This fix was tested on a Xiaomi mi i4 with Android 5.0.2 and on a Xiaomi mi Pad2 with Android 5.1.1.
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•