-
Notifications
You must be signed in to change notification settings - Fork 768
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
Adview: adapter currency fix. #2060
Conversation
2.json format tidy up.
(2)add test codes for currency.
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.
LGTM
(2)test script re-fix.
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.
LGTM.
Needs #2071 to be merged in before we can merge this one in |
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.
Looks good to me @AdviewOpen. Just one more ask: Now that pull request #2071 has been merged, can you please re-incorporate the JSON currency conversion test case you got rid of in a previous commit? JSON tests with currency conversion are now supported. As shown below, file adapters/adview/adviewtest/supplemental/currency_rate_not_found.json
covers the error scenario in line 67.
Can we also add a JSON currency test to cover lines 71 and 72?
Roger , and test script has been already in git, great thx for all of you. |
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.
LGTM. @AdviewOpen we appreciate your response to our feedback
* 'master' of https://github.com/wwwyyy/prebid-server: (23 commits) Add GPID Reserved Bidder Name (prebid#2107) Marsmedia adapter - param zoneId can get string and int (prebid#2105) Algorix: add Server Region Support (prebid#2096) New Adapter: Bizzclick (prebid#2056) Collect Prometheus Go process metrics (prebid#2101) Added channel support (prebid#2037) Rubicon: update fpd fields resolving (prebid#2091) Beachfront - Currency conversion (prebid#2078) New Adapter: Groupm (Alias Of PubMatic) (prebid#2042) Adform adapter lacked gross/net parameter support (prebid#2084) add iframe sync for openx (prebid#2094) changes to the correct user sync url (prebid#2092) Smaato: Add instl to tests (prebid#2089) Correct MakeRequests() output assertion in adapter JSON tests (prebid#2087) Adview: adapter currency fix. (prebid#2060) New Adapter: Coinzilla (prebid#2074) Syncer Level ExternalURL Override (prebid#2072) 33Across: Enable Support for SRA requests (prebid#2079) Currency conversion on adapter JSON tests (prebid#2071) New Adapter: VideoByte (prebid#2058) ...
Co-authored-by: wildcat0601 <[email protected]>
Co-authored-by: wildcat0601 <[email protected]>
for the suggestion, fixed the response currency setting.