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.

Sign up for GitHub

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

Jump to bottom

Update hello world example and remove other outdated GPT examples #4009

Merged
merged 3 commits into from
Jul 29, 2019
Merged

Update hello world example and remove other outdated GPT examples #4009

merged 3 commits into from
Jul 29, 2019

Conversation

mkendall07
Copy link
Member

Type of change

  • Refactoring (no functional changes, no api changes)

Description of change

This cleans up the GPT example pages by removing the old pattern of polling to see if GPT is ready and also removes bidder specific pages because it's not realistic to keep them all up date. Per Google, this is no longer necessary (https://developers.google.com/doubleclick-gpt/versions):

As of GPT v2019032001, Pubads service is now fully operational immediately after calling googletag.enableServices() instead of being initialized asynchronously. This means that googletag.pubadsReady is now guaranteed to be true right after calling googletag.enableServices(). Polling to check the value of googletag.pubadsReady should no longer be necessary.

@mkendall07
Copy link
Member Author

This pull request fixes 10 alerts when merging 84b0424 into d3643fd - view on LGTM.com

fixed alerts:

  • 10 for Unused variable, import, function or class

This project has automated code review enabled, but doesn't use the LGTM GitHub App. Migrate over by installing the app. Read about the benefits of migrating to GitHub Apps in the blog.

Comment posted by LGTM.com

jsnellbaker
jsnellbaker approved these changes Jul 22, 2019
View reviewed changes
Copy link
Collaborator

@jsnellbaker jsnellbaker left a comment

Choose a reason for hiding this comment

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

LGTM

@jsnellbaker jsnellbaker added the needs 2nd review Core module updates require two approvals from the core team label Jul 22, 2019
robertrmartinez
robertrmartinez approved these changes Jul 22, 2019
View reviewed changes
Copy link
Collaborator

@robertrmartinez robertrmartinez left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

integrationExamples/gpt/hello_world.html
<script>
var PREBID_TIMEOUT = 3300;
var FAILSAFE_TIMEOUT = 3300;
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 on renaming this, had heard people confused thinking it was bidder timeout and others!

@bretg
Copy link
Collaborator

bretg commented Jul 22, 2019

@mkendall07 - I made a couple of updates

  • specifically passing the PREBID_TIMEOUT into requestBids()
  • added in the FAILSAFE settimeout on the GDPR example... why was it commented out?

@mkendall07
Copy link
Member Author

This pull request fixes 10 alerts when merging 84047d6 into e6e080a - view on LGTM.com

fixed alerts:

  • 10 for Unused variable, import, function or class

This project has automated code review enabled, but doesn't use the LGTM GitHub App. Migrate over by installing the app. Read about the benefits of migrating to GitHub Apps in the blog.

Comment posted by LGTM.com

@bretg bretg assigned mkendall07 and unassigned robertrmartinez Jul 22, 2019
@bretg
Copy link
Collaborator

bretg commented Jul 22, 2019

Back to you @mkendall07 to confirm the timeout changes I made.

@mkendall07
Copy link
Member Author

LGTM

@mkendall07 mkendall07 merged commit 0bb46cf into master Jul 29, 2019
leonardlabat pushed a commit to criteo-forks/Prebid.js that referenced this pull request Jul 30, 2019
@mkendall07 @leonardlabat
Update hello world example and remove other outdated GPT examples (pr…
5b3b295 
…ebid#4009)

* Update hello world example and remove other outdated GPT examples

* updated timeouts

* updated PREBID_TIMEOUT
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 1, 2019
@mkendall07
Update hello world example and remove other outdated GPT examples (pr…
ac89563 
…ebid#4009)

* Update hello world example and remove other outdated GPT examples

* updated timeouts

* updated PREBID_TIMEOUT
sa1omon pushed a commit to gamoshi/Prebid.js that referenced this pull request Nov 28, 2019
@mkendall07
Update hello world example and remove other outdated GPT examples (pr…
fa6a4bb 
…ebid#4009)

* Update hello world example and remove other outdated GPT examples

* updated timeouts

* updated PREBID_TIMEOUT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsnellbaker jsnellbaker jsnellbaker approved these changes

@robertrmartinez robertrmartinez robertrmartinez approved these changes

Assignees

@mkendall07 mkendall07

Labels
needs 2nd review Core module updates require two approvals from the core team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mkendall07 @bretg @robertrmartinez @jsnellbaker