-
Notifications
You must be signed in to change notification settings - Fork 216
New Adapter: Sparteo #3985
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
New Adapter: Sparteo #3985
Conversation
bf1549b
to
70b88dd
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.
please make sure the Go and Java versions have feature parity before merging
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/proto/openrtb/ext/request/sparteo/ExtImpSparteo.java
Outdated
Show resolved
Hide resolved
Hi @AntoxaAntoxic , |
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
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.
the bidder itself looks good, some comments on the test class
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/test/java/org/prebid/server/bidder/sparteo/SparteoBidderTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/prebid/server/bidder/sparteo/SparteoBidderTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/prebid/server/bidder/sparteo/SparteoBidderTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/prebid/server/bidder/sparteo/SparteoBidderTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/prebid/server/bidder/sparteo/SparteoBidderTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/prebid/server/bidder/sparteo/SparteoBidderTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/prebid/server/bidder/sparteo/SparteoBidderTest.java
Outdated
Show resolved
Hide resolved
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.
Overall, looks good. Only minor issues are left. Thank you!
src/test/java/org/prebid/server/bidder/sparteo/SparteoBidderTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/prebid/server/bidder/sparteo/SparteoBidderTest.java
Outdated
Show resolved
Hide resolved
53c2d04
to
5f5206f
Compare
Hey @AntoxaAntoxic, |
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.
one minor comment, but everything else looks good
the next step is the second approval and merging after all
src/test/java/org/prebid/server/bidder/sparteo/SparteoBidderTest.java
Outdated
Show resolved
Hide resolved
5f5206f
to
c2f1822
Compare
Hi @CTMBNara, just a friendly follow-up on this PR. |
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
c2f1822
to
6bbbff7
Compare
Hi @CTMBNara, |
6bbbff7
to
4c0ad2e
Compare
src/main/java/org/prebid/server/bidder/sparteo/SparteoBidder.java
Outdated
Show resolved
Hide resolved
4c0ad2e
to
c606fab
Compare
Hi @CTMBNara |
Hi @CTMBNara and @AntoxaAntoxic , |
🔧 Type of changes
✨ What's the context?
We would like to allow our partners to be able to include us in s2s bidding.
The Go PR: prebid/prebid-server#4275
🧠 Rationale behind the change
The adapter logic is the same as the one we have implemented for the GO version
🔎 New Bid Adapter Checklist
🧪 Test plan
How do you know the changes are safe to ship to production?
We have implemented the unit and integration test.
We have run several manual "real" tests.
🏎 Quality check