Skip to content

Force-disable flag in experiments via cookies#6701

Merged
dvoytenko merged 2 commits intoampproject:masterfrom
dvoytenko:onduty7
Dec 15, 2016
Merged

Force-disable flag in experiments via cookies#6701
dvoytenko merged 2 commits intoampproject:masterfrom
dvoytenko:onduty7

Conversation

@dvoytenko
Copy link
Copy Markdown
Contributor

No description provided.

const experiments = getExperimentIds(win);

// Disabling cookie flag.
const disableFlag = experiments.indexOf('-' + experimentId) != -1;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

so this is for browser to manually opt-out?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is for a developer to manually opt-out. Basically, "We just launched feature X. Our binary config as X:1". There's no way in this scenario to override X back to 0 manually w/o redeployment. This is useful when you want to check if indeed the launch caused the problem or not.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sounds useful

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Using AMP.toggleExperiment('xxx', false) can manually opt-out for the current page view. I guess that's not enough for your use case? You want it to be persistent?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Currently it doesn't. It doesn't disable, but removes from enabled :)

const experiments = getExperimentIds(win);

// Disabling cookie flag.
const disableFlag = experiments.indexOf('-' + experimentId) != -1;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sounds useful

@dvoytenko dvoytenko merged commit 8dc24c0 into ampproject:master Dec 15, 2016
@dvoytenko dvoytenko deleted the onduty7 branch December 15, 2016 22:06
Lith pushed a commit to Lith/amphtml that referenced this pull request Dec 22, 2016
* Force-disable flag in experiments via cookies

* minus prefix
Lith pushed a commit to Lith/amphtml that referenced this pull request Dec 22, 2016
* Force-disable flag in experiments via cookies

* minus prefix
torch2424 pushed a commit to torch2424/amphtml that referenced this pull request Jan 3, 2017
* Force-disable flag in experiments via cookies

* minus prefix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants