-
Notifications
You must be signed in to change notification settings - Fork 224
Equativ: SmartAdserver alias with update to use mtype #3678
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
Equativ: SmartAdserver alias with update to use mtype #3678
Conversation
src/main/java/org/prebid/server/bidder/smartadserver/SmartadserverBidder.java
Outdated
Show resolved
Hide resolved
|
Hello everyone. I am going on vacation until February so I won't be able to react to any responses here. If this PR can wait until then, I will happily finish it in February. If not, I kindly ask @bretg to assign the Issue/PR to someone else, if any changes are required. |
…rver-parse-openrtb2-markup-type
|
@EmilNadimanov is it ready for review? |
|
@osulzhenko it is. Am I supposed to mark it with a tag or something like that? |
|
@EmilNadimanov No need—if it doesn't contain "WIP" or "draft," it will be considered ready. I was just confused by the message, which is why I decided to ask. The PR will be reviewed soon. |
src/main/java/org/prebid/server/bidder/smartadserver/SmartadserverBidder.java
Outdated
Show resolved
Hide resolved
CTMBNara
left a comment
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.
Also, please, add integration tests for new alias
|
Required update based on the comments |
|
Thanks for reviewing. I will get to it sooner or later, I am currently quite busy with our internal work. Sorry for that. |
|
@CTMBNara I've made the requested changes. Sorry that it took so long. |
CTMBNara
left a comment
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.
Rename the JSON files associated with aliases so that they contain the alias name instead of the original bidder name:
- /alias/test-smartadserver-bid-response.json
- /alias/test-smartadserver-bid-request.json
- /alias/test-auction-smartadserver-response.json
- /alias/test-auction-smartadserver-request.json
🔧 Type of changes
✨ What's the context?
Implementing #3608, i.e. porting this change from PBS-Go
🧠 Rationale behind the change
Keeping PBS-Java up-to-date
🧪 Test plan
Made changes to the fixtures in the integration test (added markupType to the Bid- and Auction Response)
Expanded/reworked unit tests for the "makeBids" method, which was affected by the changes.
🏎 Quality check
mvn validateis happy, but I might have missed something, pls let me know if I have to make any changes.