-
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
✨[Smartadserver ad extension] Implement support for Fast Fetch #36991
✨[Smartadserver ad extension] Implement support for Fast Fetch #36991
Conversation
…ver/amphtml into fast-fetch-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.
Look good w/ comments.
extensions/amp-ad-network-smartadserver-impl/0.1/amp-ad-network-smartadserver-impl.js
Outdated
Show resolved
Hide resolved
extensions/amp-ad-network-smartadserver-impl/0.1/amp-ad-network-smartadserver-impl.js
Outdated
Show resolved
Hide resolved
extensions/amp-ad-network-smartadserver-impl/0.1/amp-ad-network-smartadserver-impl.js
Outdated
Show resolved
Hide resolved
extensions/amp-ad-network-smartadserver-impl/0.1/amp-ad-network-smartadserver-impl.js
Outdated
Show resolved
Hide resolved
extensions/amp-ad-network-smartadserver-impl/0.1/amp-ad-network-smartadserver-impl.js
Outdated
Show resolved
Hide resolved
extensions/amp-ad-network-smartadserver-impl/0.1/amp-ad-network-smartadserver-impl.js
Outdated
Show resolved
Hide resolved
extensions/amp-ad-network-smartadserver-impl/0.1/amp-ad-network-smartadserver-impl.js
Outdated
Show resolved
Hide resolved
@dethstrobe, could you review the OWNERS part, please? |
Hello @rebeccanthomas, @dvoytenko, may I ask you for this extension owner approval, please? |
@newmuis I think you can approve the extension folder addition here. |
@newmuis, could you approve it, please? |
@newmuis, could you approve the extension folder? |
@smart-adserver I looked around and it appears that the majority is out for holiday. I'll circle back after the new year. |
@powerivq, do you have any news if anyone is available to approve it? |
I can provide approval. Did this go through design review? Is there an I2I filed? Generally, we do not add new extensions until these steps have been met. Sorry if I missed something or if I am out of the loop! |
@newmuis This one is a bit different. It's not a new feature extension, but an ads integration where they implement our amp-a4a skeleton (see ones starts with amp-ad-network-xxx at https://github.com/ampproject/amphtml/tree/main/extensions). It is already standardized and there is nothing to be reviewed in design for a new integration. |
Got it. Per the fast fetch implementation guide, new extensions are expected to be created, subclassing |
…nce-attr-to-hero-img * 'main' of github.com:ampproject/amphtml: (525 commits) mathml storybook: supply missing component definition. (#37326) storybook: Iframe --> BentoIframe (#37327) 🖍 [Story system layer] New ad badge (#37311) 🐛 [amp story] Replay/next page button bug fix (#37316) 🚀 [Story performance] Remove affiliate links (#37280) Compiler: Add amp-carousel-0.1 to the builder map (#37308) ⏪ [Story system layer] Audio icon disappears when story has background audio (#37314) 🚀 [Story performance] Remove story access (#37281) Fix remapping esbuild output on Windows (#37312) 🐛 adds in correct weight for amp-story-product-tag text (#37188) typechecking carousel: remove shame files (#37213) Use remapping to remap minified sourcemap into source code (#37238) SwG Release 0.1.22.199 (#37310) 🐛 Adds microsoft-edge protocol (#34168) Sync for validator cpp engine and cpp htmlparser (#37292) ✨ amp-story-shopping Updated currency with product price and correct Localized currency (#37249) ✨[Smartadserver ad extension] Implement support for Fast Fetch (#36991) Remove client-side-experiments-config.json from this repo (#37304) 🚮 Remove closure compiler logic from build-system. (#37296) 🌐 Added RTL ordering i18n for amp story shopping tag (#37252) ...
The change implements support for Fast Fetch ad rendering of the Smartadserver ad network. This introduces an extension as well as removes Smartadserver from the vendors.