Skip to content
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

Amp ad exit vendor dependency #15208

Merged

Conversation

keithwrightbos
Copy link
Contributor

amp-ad-exit has dependency on amp-analytics iframe transport iframes but is unintentionally pulling in the entire amp-analytics vendor config. Moving transport config to separate file corrected this inadvertent include reducing file size from ~50k to 12k.

@@ -26,7 +26,7 @@ Before you can add your analytics service to AMP HTML runtime, you may need to:
1. ```"vars": {}``` for additional default variables.
1. ```"requests": {}``` for requests that your service will use.
1. ```"optout":``` if needed. We currently don't have a great opt-out system, so please reach out to help us design one that works well for you.
1. If you are using iframe transport, add a new block to ANALYTICS_IFRAME_TRANSPORT_CONFIG in vendors.js containing ```"transport": { "iframe": *url* }```
1. If you are using iframe transport, add a new block to ANALYTICS_IFRAME_TRANSPORT_CONFIG in iframe-transport-vendors.js containing ```"transport": { "iframe": *url* }```
Copy link
Contributor

Choose a reason for hiding this comment

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

Change link on line 35 to:
https://github.com/ampproject/docs/blob/master/content/docs/analytics/analytics-vendors.md
(change ads_analytics to just analytics)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@keithwrightbos keithwrightbos merged commit 1187505 into ampproject:master May 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants