-
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
add optional param to bridgewellBidAdapter #2289
Conversation
[FEAT] add optional param cpmWeight to bridgewellBidAdapter
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.
Hello @wuleo ,
Overall this PR looks good, however I did see one thing that should be altered. Can you please make the change when you get the chance?
Just to mention, I had some difficulty initially testing out the changes with the hello_world page using the test params in the updated md file. It turns out the base CPM for the test ad ($0.17) coupled with the 0.5 cpmWeight
was causing the recalculated cpm to be below $0.10 - which was causing it to fall to the bottom tier price bucket which only had house ads.
Once I raised the cpmWeight
setting (to 0.75 as an example), I was consistently seeing the test ad fine. Perhaps it would be ideal to adjust the test value in the md file?
Thanks.
modules/bridgewellBidAdapter.js
Outdated
@@ -82,7 +100,8 @@ export const spec = { | |||
} | |||
|
|||
bidResponse.requestId = req.bidId; | |||
bidResponse.cpm = matchedResponse.cpm; | |||
bidResponse.requestId = req.bidId; |
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.
Can remove this duplicate line.
[REFACTOR] rm duplicate code, update md file
Some publishers in Taiwan may use this optional |
@wuleo That's fine, it'll be up to the publisher to set the value that makes sense given their needs. I just wanted to comment on test params data in case it needs to be tested later on without difficulty. The changes look good now so I'll merge them in. |
Thanks for the review. Very appreciate your effort. |
Type of change
Description of change
Add parameter
cpmWeight
that allows publisher to alter (scale) bid prices from Bridgewell.