Skip to content

An additional required file was added for Rambler&Co ad network#7541

Merged
zhouyx merged 2 commits intoampproject:masterfrom
CrazySquirrel:SSP-777-2
Feb 16, 2017
Merged

An additional required file was added for Rambler&Co ad network#7541
zhouyx merged 2 commits intoampproject:masterfrom
CrazySquirrel:SSP-777-2

Conversation

@CrazySquirrel
Copy link
Copy Markdown
Contributor

An additional required js file was added to amp-ad tag type capirs. It is necessary for some ad types in Rambler&Co ad network.

@jridgewell jridgewell requested review from lannka and zhouyx February 14, 2017 18:32
@jridgewell
Copy link
Copy Markdown
Contributor

/to @lannka, @zhouyx. XS one.

@zhouyx
Copy link
Copy Markdown
Contributor

zhouyx commented Feb 15, 2017

One question. What's the cost of window.document.write. If the overhead is expensive, maybe we should consider accepting an array of urls to write?

@CrazySquirrel
Copy link
Copy Markdown
Contributor Author

I can load the required scripts asynchronously using loadScript method. I think it will be better.

@zhouyx
Copy link
Copy Markdown
Contributor

zhouyx commented Feb 16, 2017

LGTM

@zhouyx zhouyx merged commit 3087206 into ampproject:master Feb 16, 2017
@zhouyx
Copy link
Copy Markdown
Contributor

zhouyx commented Feb 16, 2017

@CrazySquirrel Thanks for the Pull Request. Merged.

mrjoro pushed a commit to mrjoro/amphtml that referenced this pull request Apr 28, 2017
…roject#7541)

* Additional required file was added

* Asynchronous scripts loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants