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

add quantcast ID submodule #5727

Merged
merged 5 commits into from
Sep 16, 2020
Merged

add quantcast ID submodule #5727

merged 5 commits into from
Sep 16, 2020

Conversation

mckurt
Copy link
Contributor

@mckurt mckurt commented Sep 10, 2020

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

This adds quantcast user ID submodule. The submodule reads Quantcast first party cookie and passes the value to prebid bidding adapters. Currently, the submodule doesn't need any configuration parameters. The code is tested using with unit tests and userId_example.html. The code coverage on the submodule is 85.71%. Please contact asig@quantcast.com for any issues.

Link to the documentation PR: prebid/prebid.github.io#2339

mediaTypes: {
banner: {}
banner: {
sizes: [[300,250],[300,600],[728,90]]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that a recent PR tackled the same issue (#5606), but I was still getting errors if the sizes aren't defined inside the banner object. I can move this to a different PR if that'd be better.

Copy link
Collaborator

@smenzer smenzer left a comment

Choose a reason for hiding this comment

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

generally looks good to me, just a minor change in the test around cookies.

test/spec/modules/quantcastIdSystem_spec.js Outdated Show resolved Hide resolved
Copy link

@jlukas79 jlukas79 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 to me, will approve once changes above completed and conflicts resolved

@mckurt
Copy link
Contributor Author

mckurt commented Sep 15, 2020

@jlukas79 @smenzer Thanks for the review. I addressed the comment. Will attach a link to the documentation PR soon.

@mckurt
Copy link
Contributor Author

mckurt commented Sep 15, 2020

Here is the documentation PR: prebid/prebid.github.io#2339

I also updated the description.

Copy link
Collaborator

@smenzer smenzer left a comment

Choose a reason for hiding this comment

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

LGTM

@smenzer smenzer merged commit 298139f into prebid:master Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants