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 New integrating ad networks support type: streamrail #23194
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
/cc @ampproject/wg-ads @lannka |
@lannka Hi, I see that the Travis check: https://travis-ci.org/ampproject/amphtml/jobs/555752095 failed because of ".then" is forbidden. We take care of this by implementation our on pollyfill so it will support promises. |
@gershman that sounds good if you have polyfill in your own code, we can make an exemption for your case. However, to our past experience, it will be better for you to have less logic in AMP repo. I suggest you move code to your own custom JS, and have only code that handles configuration in AMP. That way you have less dependency on our release cycles to update/fix things. That way, you probably also don't need to use Promise in AMP. |
@lannka hi, I updated the code so it won't use Promise anymore. |
the e2e test failure was irrelevant |
…ject#23194) * support amp-ad type=streamrail * lint streamrail.js * amp-ad streamrail change load script
Implemented streamrail ad integration