Skip to content

Smartyads Adapter 1.x#2080

Merged
matthewlane merged 2 commits intoprebid:masterfrom
isss1650:master
Mar 6, 2018
Merged

Smartyads Adapter 1.x#2080
matthewlane merged 2 commits intoprebid:masterfrom
isss1650:master

Conversation

@isss1650
Copy link
Contributor

Type of change

  • New bidder adapter

Description of change

Added HB adapter for SmartyAds SSP

  • test parameters for validating bids
{
  bidder: 'smartyads',
  params: {
        placementId: 0
  }
}

@isss1650 isss1650 changed the title Smartyads Adapter 1.0 Smartyads Adapter 1.x Jan 29, 2018
@bretg bretg requested a review from pm-harshad-mane January 30, 2018 15:29
@bretg
Copy link
Contributor

bretg commented Jan 30, 2018

Thanks for the submission @isss1650. While the review is taking place, please note that we will require that a file for this adapter be added to https://github.com/prebid/prebid.github.io/tree/master/dev-docs/bidders

@mkendall07 mkendall07 added this to the Prebid 1.4.0 milestone Feb 12, 2018
@isss1650
Copy link
Contributor Author

Docs
prebid/prebid.github.io#624

@mkendall07 mkendall07 self-requested a review February 20, 2018 19:14
@mkendall07 mkendall07 self-assigned this Feb 20, 2018
@mkendall07 mkendall07 modified the milestones: Prebid 1.4.0, Prebid 1.5.0 Feb 23, 2018
}
switch (bid['mediaType']) {
case 'banner':
return Boolean(bid['width'] && bid['height'] && bid['ad']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need the Boolean function here. Also prefer dot notation bid.width vs bracket notation for known keys.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dot notation added.
Boolean - disagree. In my opinion, the function must always return data of the same type.
The validation function must return a value in the boolean data type.
Don't you agree with me?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that's fine I'm just use to reviewing code with !! instead of Boolean but it should be functionally equivalent.

return false;
}
switch (bid['mediaType']) {
case 'banner':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have constants for these types you can use.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

@mkendall07 mkendall07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One comment, your user sync URL http://ssp-nj.webtradehub.com/?c=o&m=cookie returns a 404

@matthewlane matthewlane merged commit b82f852 into prebid:master Mar 6, 2018
mizmaar3 added a commit to widespace-os/Prebid.js that referenced this pull request Mar 8, 2018
* master: (27 commits)
  Increment pre version
  Prebid 1.5.0 Release
  Fix cross-platform test failures (prebid#2228)
  Fix uncahced video bids from multi-response array triggering callback early (prebid#2219)
  Add vuble adapter (prebid#2201)
  Update Vidazoo domain (prebid#2223)
  InSkin Bid Adapter: remove referrer field from request body (prebid#2217)
  Gamma Support UserSync Endpoint (prebid#2216)
  Feature/stale bot (prebid#2192)
  33Across Bid Adapter: updated user sync endpoint (prebid#2193)
  Adding PR_REVIEW guideline (prebid#2159)
  Add FairTrade Bid Adapter (prebid#2147)
  Add banner support to Beachfront adapter (prebid#2117)
  Smartyads Adapter 1.x (prebid#2080)
  Audience Network: allow native bids for non-IAB sizes (prebid#2203)
  Update position default value in rubicon (prebid#2214)
  Auctionmanager spec refactor pr (prebid#2155)
  fix mediaType being overwritten by undefined in rubicon bid adapter (prebid#2209)
  Fix lint error (prebid#2208)
  VAST support in adform adapter (prebid#2173)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Comments