-
Notifications
You must be signed in to change notification settings - Fork 760
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
Criteo: Add support for native #3709
Conversation
MarinaZhuravlevaCriteo
commented
May 29, 2024
- Added native media type in list of capabilities for criteo.yaml
- Added test for simple native case for criteo adapter
Hello, Please could you review this ? Thanks! |
Hello team Small up on this PR that has been opened for a while now 🙏 Many thanks |
}, | ||
"ext": { | ||
"bidder": { | ||
"uid": 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: it doesn't effect the test behavior but uid
doesn't appear to be a valid bidder parameter for your bidder. It might be best to specify a real bidder param here like zoneid
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just updated the chance, using uid here was not the intent
6a6a5ee
to
08b59d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For future reference, once the review cycle has started, please avoid force pushing and instead push up new commits (or merge with master if you need to resolve merge conflicts) as it makes it much easier for reviewers as we just need to review the delta instead of re-reviewing the entire PR.
All of the commits are squashed when we merge.
Thanks!