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

Underdog Media 1.0 Compliance #1801

Merged
merged 2 commits into from
Nov 8, 2017
Merged

Underdog Media 1.0 Compliance #1801

merged 2 commits into from
Nov 8, 2017

Conversation

Jacobkmiller
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
  • [ x] Other

Description of change

Update of Underdog Media adapter for 1.0 Compliance

@Jacobkmiller Jacobkmiller changed the title updated underdogmedia adapter for prebid 1.0 Underdog Media 1.0 Compliance Nov 3, 2017
@jaiminpanchal27 jaiminpanchal27 self-assigned this Nov 7, 2017
@mkendall07 mkendall07 closed this Nov 7, 2017
@mkendall07 mkendall07 reopened this Nov 7, 2017
Copy link
Collaborator

@jaiminpanchal27 jaiminpanchal27 left a comment

Choose a reason for hiding this comment

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

@Jacobkmiller Thanks for updating adapter.
Left some minor comments.


function _makeNotification(bid, mid, bidParam) {
makeNotification: function (bid, mid, bidParam) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

makeNotification and getUrlVars function can remain private in adapter. No need to added them to spec

return {
method: 'GET',
url: `${window.location.protocol}//udmserve.net/udm/img.fetch`,
data: `tid=1;dt=10;sid=${siteId};sizes=${sizes.join(',')}`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

More params can be added to request object here. So you can add bidParams to bidRequest and access that in interpretResponse method.
By doing this you can remove bidParams line 15.

siteId: '12143'
},
requestId: '10b327aa396609',
placementCode: '/123456/header-bid-tag-1'
Copy link
Collaborator

Choose a reason for hiding this comment

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

In 1.0 we have renamed placementCode to adUnitCode. Please rename

@Jacobkmiller
Copy link
Contributor Author

Thanks @jaiminpanchal27 for the review. I have updated the PR with your changes.

@jaiminpanchal27 jaiminpanchal27 merged commit cf89650 into prebid:master Nov 8, 2017
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