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
Adds TemplateValidator #14584
Adds TemplateValidator #14584
Conversation
78d9838
to
312ac42
Compare
import {utf8Decode} from '../../../src/utils/bytes'; | ||
|
||
/** @const {string} */ | ||
export const AMP_TEMPLATED_CREATIVE_HEADER_NAME = 'AMP-template-amp-creative'; |
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.
As stated in the design doc, we'd like to rename this to AMP-Ad-Template-Extension
. Meantime, please make sure the we don't break AdZerk. So we might end of having both, until AdZerk getting migrated over.
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.
Done.
creativeData: { | ||
creative: body, | ||
}, | ||
adResponseType: 'template', |
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.
is this an enum?
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.
Fixed.
268e130
to
422a42c
Compare
What's with the presubmit error: "Please review viewport service for helper methods or mark with |
import {utf8Decode} from '../../../src/utils/bytes'; | ||
|
||
/** @const {string} */ | ||
export const AMP_TEMPLATED_CREATIVE_HEADER_NAME = 'AMP-Ad-Template-Extension'; |
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.
As I mentioned, can we accept both headers for now? So later we can easily migrate AdZerk over.
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.
Done. Sorry about the confusion.
For your presubmit error: the use of preloadExtension needs to be whitelisted: https://github.com/ampproject/amphtml/blob/master/build-system/tasks/presubmit-checks.js#L698 |
Addresses #15464. |
No description provided.