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.

Sign up for GitHub

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

Jump to bottom

AdKernelAdn adapter #1747

Merged
merged 1 commit into from
Nov 7, 2017
Merged

AdKernelAdn adapter #1747

merged 1 commit into from
Nov 7, 2017

Conversation

ckbo3hrk
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

  • test parameters for validating bids
{
  bidder: 'adkernelAdn',
  params: {
    pubId: 50357,
    host: 'dsp-staging.adkernel.com'
  }
}
  • contact email of the adapter’s maintainer: [email protected]
  • official adapter submission

@ckbo3hrk ckbo3hrk force-pushed the adkernelAdn branch 3 times, most recently from e1f1c0c to 479dacd Compare October 26, 2017 15:54
@jaiminpanchal27 jaiminpanchal27 self-assigned this Nov 1, 2017
jaiminpanchal27
jaiminpanchal27 approved these changes Nov 1, 2017
View reviewed changes
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.

LGTM

@jaiminpanchal27 jaiminpanchal27 removed their assignment Nov 3, 2017
matthewlane
matthewlane reviewed Nov 3, 2017
View reviewed changes
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.

One requested change then this should be good for merge

modules/adkernelAdnBidAdapter.js Outdated
id: bidRequest.bidId,
tagid: bidRequest.placementCode
};
if (bidRequest.mediaType === 'video') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should also check for the new mediaTypes property introduced in #1483 (with something like || (bidRequest.mediaTypes && bidRequest.mediaTypes.video) or however you'd prefer) in case a pub defines a video adunit with mediaTypes: {video: {}} rather than mediaType: 'video'

@ckbo3hrk
Copy link
Contributor Author

ckbo3hrk commented Nov 7, 2017

@matthewlane Thanks for code review. I've made proposed changes.

@matthewlane matthewlane merged commit a2b5cfa into prebid:master Nov 7, 2017
@ckbo3hrk ckbo3hrk deleted the adkernelAdn branch November 2, 2020 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaiminpanchal27 jaiminpanchal27 jaiminpanchal27 approved these changes

@matthewlane matthewlane matthewlane approved these changes

Assignees

@matthewlane matthewlane

Labels
LGTM Prebid 1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ckbo3hrk @matthewlane @jaiminpanchal27