-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Force upgrade sticky ad to 1.0 #9042
Conversation
Why are we updating only tests? How would the actual upgrade happen? Are you saying that you are deploying 1.0 to 0.1 instead? |
Yes. Use the file |
LGTM, but please wait for @erwinmombay |
ping @erwinmombay |
gulpfile.js
Outdated
@@ -378,6 +378,10 @@ function buildExtension(name, version, hasCss, options, opt_extraGlobs) { | |||
options = options || {}; | |||
options.extraGlobs = opt_extraGlobs; | |||
var path = 'extensions/' + name + '/' + version; | |||
if (name == 'amp-sticky-ad' && version == '0.1') { | |||
// sticky-ad upgrade, 0.1 is deprecated. |
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.
please add explanation why we do this
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.
one request to add explanation
gulpfile.js
Outdated
if (name == 'amp-sticky-ad' && version == '0.1') { | ||
// Special case for sticky-ad upgrade. | ||
// To deprecate 0.1, replace the build path so that amp-sticky-ad-0.1.js | ||
// is built from extensions/amp-sticky-ad/1.0/amp-sticky-ad.js |
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.
can you explain further "why" we want this.
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.
how do you like the explanation now : )
For #6169
cc @jasti
I will remove the actually file once the upgrade is stable in prod.