-
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
integr8 Bid Adapter: add new bid adapter #6882
Conversation
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 @ardit-baloku,
You set once your adRequest params, so if there are several adUnits using your bidder with different params, only the first value found will be set. Could you confirm it is the expected behavior?
You need to support the FloorPrices module. It is required with Prebid.js 5.x.
Regarding the video:
- you must read and pass to your server the optional video params defined by the publisher at the adUnit level (Support video params at the adunit level #6512)
- you must provide a renderer for outsream context.
See https://docs.prebid.org/dev-docs/bidder-adaptor.html#supporting-video for more information.
Hey there, any news @ardit-baloku ? |
Hi @osazos, sorry for the late response, I was on vacation. I have resolved the comments you left on this PR. Regarding your question if the expected behaviour was to use the first found value, the answer is yes we intended that. I have also added support for the FloorPrices module as you requested. Regarding video, we only used "skipTime" as a parameter for video ads and we are now fetching that from bidRequest.mediaTypes.video.skipafter, but I have left bidRequest.params.skipTime as a fallback to keep it backwards compatible. Do we need to provide a renderer outstream context? We do not plan on supporting this any time soon and would like to skip this part if possible |
Hi, sorry the delay. |
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.
LGTM
@ChrisHuie, do you want to review it or you just self-added yourself by mistake?
@osazos Added myself just so it wasn't sitting assigned to you while it wasn't moving :) Looks good to me! |
* Added integr8 adapter * Double quote to single quote * Added floor module support * Added floor tests
Type of change
Description of change
Added new adapter for integr8 adplatform