-
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 new adapter districtmDMX #811
Conversation
test @97%, two files added one updated
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.
Thanks for the PR. Two requested changes noted below
it('it\'s now time to play with the response ...', ()=>{ | ||
let result = districtm.handlerRes(PREBID_RESPONSE(), PREBID_PARAMS); | ||
_each(result, function(k, v){ | ||
console.log(11, `${k} value is ${v}`); |
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.
In the test
directory we try to avoid logging to the console to avoid having non-build related activity displayed during the build process. Please convert this to an assertion if it makes sense or remove the line
|
||
var DistrictmAdaptor = function districtmAdaptor(){ | ||
let districtmUrl = window.location.protocol + '//prebid.districtm.ca/lib.js'; | ||
this.callBids = params =>{ |
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.
The spacing on this line 6 and this line is causing a jscs error: Expected indentation of 2 characters
. Please use two spaces on these lines rather than 3, or alternatively, place // jscs:disable
at the top of this file to avoid these errors from displaying during the build process
I just make some change please review |
Thanks for the update. It looks like this adapter is making a call to the same /jpt endpoint as the AppNexus adapter, any reason to not alias the AppNexus adapter instead? |
Yes, we are adding addtional features such as the currency parameter which allows us to send the bid in foreign currency in the ad server. We take care of updating currencies on a daily basis on behalf of the publisher |
@stevealliance |
If the publisher has an ad server setup in Canadian as an example, we apply convertion to it. Many publishers are having Canadian ad servers, we tought this feature would be an option to simplify the process for them. They have to be aware that this have to be used in a context of comparing apples to apples. A little bit like gross vs net. Thanks |
…ebid-official-0.16.0 to release/1.10.0 * commit 'fd16420d7034b1f82e571e2b122b7fa73b88d929': Add changelog entry. Add new adapters for AOL analytics. Updated Prebid version Catch cross-origin DOMException (prebid#861) Add GumGum adapter (prebid#833) Remove duplicate log line in request bids (prebid#859) Utility function getBidIdParamater is misspelled Allow Conversant sizes to be overridden per placement (prebid#816) Add districtmDMX adapter (prebid#811) Truncate bids requested on clearPlacements (prebid#825) (prebid#828) Update adapters.json (prebid#803) Add usersyncing to AppNexus adapters (prebid#845) fixes for bugs in test suite (prebid#810) Pass user object parameters on bid request (prebid#821) fix rubicon deals to be per ad instead of per response (prebid#838) Links bidId in request with adId in response for analytics purposes (prebid#836) Bugfix/issue building from npm (prebid#823) Update build to only run `webpack` to improve build time performance (prebid#809) Cast all Conversant site_ids to a string (prebid#829) Increment pre version
…10.0 to master * commit 'b39f2b12a8ddfa650a8e04e3abd358e60371950a': Add changelog entry. Add new adapters for AOL analytics. Updated Prebid version Catch cross-origin DOMException (prebid#861) Add GumGum adapter (prebid#833) Remove duplicate log line in request bids (prebid#859) Utility function getBidIdParamater is misspelled Allow Conversant sizes to be overridden per placement (prebid#816) Add districtmDMX adapter (prebid#811) Truncate bids requested on clearPlacements (prebid#825) (prebid#828) Update adapters.json (prebid#803) Add usersyncing to AppNexus adapters (prebid#845) fixes for bugs in test suite (prebid#810) Pass user object parameters on bid request (prebid#821) fix rubicon deals to be per ad instead of per response (prebid#838) Links bidId in request with adId in response for analytics purposes (prebid#836) Bugfix/issue building from npm (prebid#823) Update build to only run `webpack` to improve build time performance (prebid#809) Cast all Conversant site_ids to a string (prebid#829) Increment pre version
@stevealliance/ @mkendall07 - I'm upgrading prebid from 0.24. to 1.10 and don't see an adapter listed in the modules folder for Can you advise if this adapter is still supported and the proper module I should be including in my build. FWIW, I did compile prebid with all of the adapters and still received the same message. |
Can someone answer @internetchris's question? And I have the same issue right now. Thanks! |
@stevealliance Any news on if there will be a DistrictmDMX module compatable with > 1.10? We are currently performing an (overdue) upgrade from PB 0.21 and while DistrictmDMX is still listed in the official list of bidders, there is no districtmDMXBidAdapter.js file under /modules. Are you the point person for this update or is the community of devs supposed to chime in? We have several properties that use DistrictM and don't want to push our upgrade live until we get them working again. @matthewlane |
@lizardslair I have a PR in review for next release #2765 Thanks |
Type of change
Description of change
added districtmDMX to adapters.json
added districtmDMX.js to folder src/adapters
added districtm_specs.js to test for unit testing
Other information
test @97%, two files added one updated