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

Stub adloader across the board to prevent requests going out #3196

Merged
merged 2 commits into from
Oct 17, 2018

Conversation

snapwich
Copy link
Collaborator

Type of change

  • Refactoring (no functional changes, no api changes)

Description of change

Pulled out the adloader stubbing into a global mock so that it is stubbed by default. Modules that need to use the actual adloader implementation (like adloader_spec.js) can access the original through the mock module.

Copy link
Collaborator

@jaiminpanchal27 jaiminpanchal27 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@mkendall07 mkendall07 left a comment

Choose a reason for hiding this comment

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

Dumb question, but why can't we just delete adLoader.js ? We already deprecated it right...

@jaiminpanchal27
Copy link
Collaborator

That is the goal as mentioned in the description #2779 by @jsnellbaker. Jason did some cleanup.

@snapwich
Copy link
Collaborator Author

@mkendall07 I'm not sure on the status of adloader.js as a whole, however it only exports two functions (loadScript and loadExternalScript) and of the two, only one of those is marked as deprecated

@mkendall07
Copy link
Member

alright well the work is already done here so I'm good with merging. Eventually we should delete it. Thanks!

@snapwich snapwich merged commit 73e7014 into master Oct 17, 2018
@snapwich snapwich deleted the global-adloader-stub branch October 17, 2018 21:41
pedrolopezmrf pushed a commit to Marfeel/Prebid.js that referenced this pull request Mar 18, 2019
…3196)

* stub adloader across the board to prevent requests going out

* add the adloader stub
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants