Conversation
mkendall07
left a comment
There was a problem hiding this comment.
Thanks for the PR. Please address code review comments.
src/adapters/adkernel.js
Outdated
| @@ -0,0 +1,194 @@ | |||
| var bidmanager = require('../bidmanager.js'); | |||
There was a problem hiding this comment.
please use ES 6 import syntax here.
src/adapters/adkernel.js
Outdated
| * Adapter for requesting bids from AdKernel white-label platform | ||
| * @class | ||
| */ | ||
| var AdkernelAdapter = function AdkernelAdapter() { |
src/adapters/adkernel.js
Outdated
| const dispatcher = new RtbRequestDispatcher(); | ||
| //process individual bids | ||
| utils._each(bids, function (bid) { | ||
| let size = bid.sizes[0]; |
There was a problem hiding this comment.
Does your adapter support multi size?
There was a problem hiding this comment.
No, adapter doesn't support multiple sizes.
src/adapters/adkernel.js
Outdated
| * Create bid object for the bid manager | ||
| */ | ||
| function createBidObject(bidObj, width, height) { | ||
| return utils.extend(bidfactory.createBid(1), { |
There was a problem hiding this comment.
Please store a reference to the bid request object and pass it to createBid(1, bidRequest) as this allows us to match the bid.adId to bid response.
test/spec/adapters/adkernel_spec.js
Outdated
| sizes: [[728, 90]] | ||
| }; | ||
|
|
||
| const bidResponse1 = { |
There was a problem hiding this comment.
nit: please apply formatting to spec file.
| //translate adunit info into rtb impression dispatched by host/zone | ||
| this.addImp = function (host, zone, width, height, bidId, adUnitId) { | ||
| if (!(host in _dispatch)) { | ||
| _dispatch[host] = {}; |
There was a problem hiding this comment.
If host is undefined, you should abort this request. Without the host param, I was seeing network requests to http://undefined/....
src/adapters/adkernel.js
Outdated
|
|
||
| function buildRequestParams(zone, rtbReq) { | ||
| return { | ||
| 'zone': zone, |
There was a problem hiding this comment.
zone should be encoded too if possible.
|
@ckbo3hrk |
|
@mkendall07 |
|
@mkendall07 |
src/adapters/adkernel.js
Outdated
| _dispatch = {}, | ||
| const _dispatch = {}, | ||
| originalBids = {}, | ||
| site = createSite(); |
There was a problem hiding this comment.
Please change to:
const _dispatch = {};
const originalBids = {};
const site = createSite();
src/adapters/adkernel.js
Outdated
| @@ -0,0 +1,207 @@ | |||
| import bidmanager from "src/bidmanager"; | |||
There was a problem hiding this comment.
please use single quotes for string literals '
mkendall07
left a comment
There was a problem hiding this comment.
LGTM. Can merge when you make those minor changes I have commented on.
|
@ckbo3hrk if you get this done today, it will be included in the 0.15.0 release. |
|
@mkendall07 |
|
LGTM. Bids are being received, unit tests pass. Thanks for contributing to Prebid.js |
|
@ckbo3hrk are all parameter's required or are there optional inputs? |
|
@aneuway2 |
|
@ckbo3hrk |
…ebid-0.15.2 to release/1.8.0 * commit '3c3d298eb248a7f69dd4ae86f3f6341f2c04b1b5': (36 commits) CHANGELOG Add new adapters for AOL analytics. Prebid 0.15.2 Release Fix issue with building prebid from npm install. Babel loader was being skipped because of the project residing in /node_modules/ directory. (prebid#820) Increment pre version Prebid 0.15.1 Release Update bidFloor->bidfloor to match other bidders + updated conversant docs (prebid#818) Correct 'bidloor' typo in Conversant adapter (prebid#815) Increment pre version 0.15.0 release support IE userLanguage Navigator property (prebid#807) exclude node_modules from babel-loader as to prevent strict mode errors (prebid#806) PulsePoint Adapter - Support for additional parameters. (prebid#784) Fix 'skippable' targeting typo (prebid#804) Adding adKernel adapter (prebid#739) Babel plugin fixes to make the generated code syntax valid in ie8 (prebid#799) Vertoz Adapter (prebid#789) RubiconLite is now just Rubicon and "rubicon" aliases "rubiconLite" (prebid#783) add version to global namespace (prebid#794) Adform adapter tests fixed prebid#769 (prebid#793) ...
…8.0 to master * commit '997aa5bce17aafd6b215d335551e89757f92bf3e': (37 commits) CHANGELOG. CHANGELOG Add new adapters for AOL analytics. Prebid 0.15.2 Release Fix issue with building prebid from npm install. Babel loader was being skipped because of the project residing in /node_modules/ directory. (prebid#820) Increment pre version Prebid 0.15.1 Release Update bidFloor->bidfloor to match other bidders + updated conversant docs (prebid#818) Correct 'bidloor' typo in Conversant adapter (prebid#815) Increment pre version 0.15.0 release support IE userLanguage Navigator property (prebid#807) exclude node_modules from babel-loader as to prevent strict mode errors (prebid#806) PulsePoint Adapter - Support for additional parameters. (prebid#784) Fix 'skippable' targeting typo (prebid#804) Adding adKernel adapter (prebid#739) Babel plugin fixes to make the generated code syntax valid in ie8 (prebid#799) Vertoz Adapter (prebid#789) RubiconLite is now just Rubicon and "rubicon" aliases "rubiconLite" (prebid#783) add version to global namespace (prebid#794) ...
…_upgrade priceFloors: fix bug where floors are not set when TIDs are disabled …
Type of change
Description of change
Added AdKernel adapter
Other information