-
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
Eplanning Bid Adapter: add support for video #9044
Conversation
@fndigrazia looks like there are some unit tests failing if you can check. |
Hello @ChrisHuie, thanks for creating the pr. The tests failed because you have to delete line 121 "ad: ad.adm,". In the original pr that line was deleted. |
Fixed. Thanks 🙏 |
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
function cleanName(name) { | ||
return name.replace(/_|\.|-|\//g, '').replace(/\)\(|\(|\)|:/g, '_').replace(/^_+|_+$/g, ''); | ||
} | ||
|
||
function getSpaces(bidRequests, ml) { | ||
let impType = bidRequests.reduce((previousBits, bid) => (bid.mediaTypes && bid.mediaTypes[VIDEO]) ? (bid.mediaTypes[VIDEO].context == 'outstream' ? (previousBits | 2) : (previousBits | 1)) : previousBits, 0); |
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.
Not required, but for better readability, would you want to replace the part
bid.mediaTypes[VIDEO].context == 'outstream' ? (previousBits | 2) : (previousBits | 1)
by
bid.mediaTypes[VIDEO].context == 'outstream' ? (previousBits | VAST_OUTSTREAM) : (previousBits | VAST_INSTREAM)
* add video * move up imptypes logic * fix linting * update curly braces * fix spaces linting * fix line
* add video * move up imptypes logic * fix linting * update curly braces * fix spaces linting * fix line
Resubmit of pr #9020