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
📖 Fix experiment-related sample code in documentation #21031
Conversation
…te more similar to the original spirit of the example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RE: the other PR, I think it should be userAssert, like in other live extensions with experiments enabled.
amphtml/extensions/amp-viz-vega/0.1/amp-viz-vega.js
Lines 74 to 76 in 81f0c14
buildCallback() { | |
userAssert(isExperimentOn(this.win, 'amp-viz-vega'), | |
'Experiment amp-viz-vega disabled'); |
I'm not sure why the generator and documentation say that a warn is sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
* Replace user.warn() with userAssert in accordance with gulp test checks * Replace userAssert() with correct usage of user().warn() to make update more similar to the original spirit of the example * Move comment to new line * manual lint * Replace user().warn() with userAssert
* Replace user.warn() with userAssert in accordance with gulp test checks * Replace userAssert() with correct usage of user().warn() to make update more similar to the original spirit of the example * Move comment to new line * manual lint * Replace user().warn() with userAssert
building-an-amp-extension.md
provides experiment-related code snippets that cause Travis checks to fail.this.getWin()
withthis.win
and incorrectuser.warn()
withuserAssert()
EXPERIMENT
in calls toisExperimentOn()
with its string literal equivalent (a required change due to check failures)