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

Dxkulture Bid Adapter: changing default video placement value #10473

Merged
merged 3 commits into from
Sep 27, 2023

Conversation

dani-nova
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

  • Does this change affect user-facing APIs or examples documented on http://prebid.org?

  • Other

Description of change

Changing default video placement value

@@ -329,7 +329,7 @@ function buildVideoRequestData(bidRequest, bidderRequest) {

// Placement Inference Rules:
// - If no placement is defined then default to 1 (In Stream)
video.placement = video.placement || 2;
video.placement = video.placement || 1;
Copy link
Collaborator

@patmmccann patmmccann Sep 12, 2023

Choose a reason for hiding this comment

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

video.placement is deprecated; why not also support plcmt? The reason for the deprecation was actually logic like this; the buy side described instream as over-specified.

Also pls cover in unit tests; this should have broken an existing test. The fact that it didnt shows your adapter is under-covered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@patmmccann We are still operating on openrtb 2.5. We made suggested changes

@ChrisHuie ChrisHuie self-assigned this Sep 21, 2023
@ChrisHuie ChrisHuie merged commit e0f80a4 into prebid:master Sep 27, 2023
2 checks passed
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