Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust Edge comparison SEM page experiment (Fixes #6371) #6442

Merged
merged 1 commit into from Nov 8, 2018

Conversation

alexgibson
Copy link
Member

@alexgibson alexgibson commented Nov 6, 2018

Description

  • Update default template for /firefox/new/?xv=edge to use what was ?v=2.
  • Adjust traffic cop experiment to use a 50/50 split for traffic, using ?v=3 as the new variation to test against control.

Issue / Bugzilla link

#6371

Testing

  • Experiment still runs as expected.
  • Correct template is shown for default /firefox/new/?xv=edge URL.

@alexgibson alexgibson added P3 Third level priority - Nice to have Review: XS Code review time: up to 30min Do Not Merge ⚠️ WIP 🚧 Pull request is still work in progress labels Nov 6, 2018
@alexgibson alexgibson removed Do Not Merge ⚠️ WIP 🚧 Pull request is still work in progress labels Nov 7, 2018
@alexgibson alexgibson added Review: S Code review time: 30 mins to 1 hour and removed Review: XS Code review time: up to 30min labels Nov 7, 2018
@@ -46,9 +46,8 @@
var cop = new Mozilla.TrafficCop({
id: 'experiment_firefox_new_edge',
variations: {
'xv=edge&v=a': 33, // control
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's where the double xvs have been coming from 🤦‍♀️.

@@ -46,9 +46,8 @@
var cop = new Mozilla.TrafficCop({
id: 'experiment_firefox_new_edge',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update the experiment ID for saftey's sake. I doubt we'll see many any users returning through a paid media buy like this but if they do we don't want traffic cop sending them back to v=1 because that's in their stored cookie.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice call 👍

Copy link
Contributor

@stephaniehobson stephaniehobson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm glad you were able to make sense of my comment in the issue :) Looks good. I recommend one small change the the id just to be through.

@alexgibson
Copy link
Member Author

Thanks @stephaniehobson updated

@stephaniehobson stephaniehobson merged commit a34beda into mozilla:master Nov 8, 2018
@alexgibson alexgibson deleted the sem-test-adjust branch November 15, 2019 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Third level priority - Nice to have Review: S Code review time: 30 mins to 1 hour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants