Audiencelogy Bid Adapter : Initial Release #11853
Conversation
|
Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:
Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀 |
|
Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:
Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀 |
|
Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:
Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀 |
modules/audiencelogyBidAdapter.js
Outdated
| dealId: bid && bid.dealId ? bid.dealId : undefined, | ||
| ttl: 300 | ||
| const buildResponse = (bid) => { | ||
| let response = {}; |
There was a problem hiding this comment.
We can't accept changes designed to fool the duplication checker
There was a problem hiding this comment.
I understand, we are an advertising tech company helping different clients like relevate health. We are fine with adding common code, please let me know where to add common functions in library?
|
Please add docs to -> https://github.com/prebid/prebid.github.io |
|
modules/audiencelogyBidAdapter.js
Outdated
| const setUser = (bid) => { | ||
| let user = {}; | ||
| if (bid && bid.params) { | ||
| user.buyeruid = localStorage.getItem('adx_profile_guid') ? localStorage.getItem('adx_profile_guid') : ''; |
There was a problem hiding this comment.
You have to import the storage manager to do this, publishers manage bidder access to storage depending on consent or their preferences
modules/audiencelogyBidAdapter.js
Outdated
| // Function to fetch bid_floor | ||
| const fetchFloor = (bid) => { | ||
| if (bid.params && bid.params.bid_floor) { | ||
| return bid.params.bid_floor; |
There was a problem hiding this comment.
Make sure to check the getFloor function from the request
|
Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:
Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀 |
|
Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:
Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀 |
|
Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:
Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀 |
|
Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:
Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀 |
|
@patmmccann I have added library audiencelogyUtils/bidderUtils.js and imported common code. Please verify. |
|
@hasanideepak yes but you should also import in relevate, which has the same concern above, it uses local storage without importing storage manager |
There are two different fork repo for relevate and audiencelogy. I will need to add same library in relevate fork in order to use it, doesnt that give conflict error as same code will be there in audiencelogy and relevate? |
both will import it, but the build will be more compact as the code will only be in the project once. It will clear the error.
Prebid.js/modules/ixBidAdapter.js Line 110 in ca24c39 What you have in relevate is a major rules violation, as it ignores consent. We need you to take care of it in this commit. I'll go ahead and submit a bug fix to delete what you are doing there now, so you'll want to merge in master after that merges to avoid merge conflicts |
@patmmccann Thank you for the support. So, I will make the changes for store manager in this PR (i.e.audiencelogy) and create a new PR for relevate adapter to import common library and store manager changes. Is this right ? |
|
Just do it all in one? |
|
@patmmccann Considering we are an ad tech service provider to relevate and we also need our own prebid adapter, we would like to focus on relevate adapter at this moment to make it fully compliant with your policies (with storage manager changes), hence we would like to pause work on our own adapter (i.e. audiencelogy) at this moment and we will revisit in a few weeks. |
|
No worries, just import storage manager instead of directly accessing local storage, keeping an eye out for your commit |
Type of change
Bugfix
Feature
New bidder adapter
Updated 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
Other information