Skip to content
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

Update sharethrough bid adapter #1740

Merged
merged 4 commits into from
Nov 3, 2017

Conversation

chriscpan
Copy link
Contributor

Type of change

  • Bugfix
  • Feature
  • New bidder adapter
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other

Description of change

Sharetrough bid adapter update for Prebid 1.0

Copy link
Collaborator

@matthewlane matthewlane left a comment

Choose a reason for hiding this comment

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

Thanks for the updated adapter. Please also add your .md file in this PR: http://prebid.org/dev-docs/bidder-adapter-1.html#planning-your-adapter

@chriscpan
Copy link
Contributor Author

Sorry about that, I updated the PR with the .md file. Thanks for the review

@matthewlane matthewlane self-assigned this Nov 1, 2017
Copy link
Collaborator

@matthewlane matthewlane left a comment

Choose a reason for hiding this comment

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

Looks good. There were a few breaking changes to 1.0 adapter structures recently, I noted fixes for them below and then we should be good to merge


return [{
requestId: req.data.bidId,
bidderCode: BIDDER_CODE,
Copy link
Collaborator

Choose a reason for hiding this comment

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

bidderCode will be set automatically by bidderFactory now, this line can be dropped

};
})
},
interpretResponse: (resp, req) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

#1748 changed the first argument of interpretResponse to:

{
  body: responseBody,
  headers: {
    get: function(header) { /* returns a header from the HTTP response */ }
  }
}

so adding something like

resp = resp.body;

just below this line, or however you'd prefer to grab the body, and updating corresponding tests if needed should get this back to working properly

@chriscpan
Copy link
Contributor Author

Awesome, good to know. Ready for review

@chriscpan chriscpan force-pushed the cp/str-adapter-update branch 2 times, most recently from 580181a to 381161d Compare November 3, 2017 20:40
@matthewlane matthewlane merged commit a1678d3 into prebid:master Nov 3, 2017
@jchau87 jchau87 deleted the cp/str-adapter-update branch June 21, 2018 20:50
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
* Update sharethrough bid adapter

* add md for sharethroughBidAdpapter

* remove bidderCode and parse response

* remove linting errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants