-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Discovery Bid Adapter: add adUnitCode, gpid, and adslot #10615
Conversation
Hello, I am the new developer of Discovery Bid Adapter, there are two checks in my submission that have not automatically run and need to be approved, thank you. |
Hello, I am the new developer of Discovery Bid Adapter, there are two checks in my submission that have not automatically run and need to be approved, thank you. |
Hi, there are two checks in my submission that have not automatically run and need to be approved. Can you please help me with approval and review?thank you @patmmccann 🙌 |
Hi, this PR is important to us. Could you please review it? Thank you very much. @peixunzhang @ChrisHuie 🙌 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These change are mainly to get more parameters
@ecoeco163 catching up on prs. We had our Prebid Summit this week so most folks have been out this week and are just getting back. Should get reviewed over the next couple of days. Can you please add tests 🙏 |
Hi, I have added tests. Could you please review it again? Thank you very much. 🙏 @ChrisHuie |
* | ||
* @returns {string} eg: "widthxheight" | ||
*/ | ||
function getScreenSize() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have a parseSizesInput()
utils function that would be more stable in the event multiple sizes are passed
https://github.com/prebid/Prebid.js/blob/master/src/utils.js#L136
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thank you for your suggestion, I used parseSizesInput utils function to handle ScreenSize. Could you please review it again? Thank you very much. 🙏 @ChrisHuie
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, The PR has passed all the tests, Could you please approve it? I'm very sorry for being so urgent, but it's really important to us. Thank you @ChrisHuie
Type of change
Bugfix
Feature
New bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other
Description of change
When initiating a bid request, new parameters including adUnitCode, adslot, gpid, and token have been added.
Other information