-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Updated Sovrn Bid Adaptor for MultiSized and added Error Call Home. #3237
Conversation
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 haven't had time to do a full review but saw two issues on my quick review
I have addressed existing comments in the most recent commit. Mike if you are too busy to do this review, would it help if I split multi-sized support and error tracking into two PR's? |
This pull request introduces 1 alert when merging f165cb0 into 2c5685c - view on LGTM.com new alerts:
Comment posted by LGTM.com |
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, not sure why the circleci test failed
So, is there something I need to do to get this merged? Please let me know if there is anything I can do to help move this forward. |
@jrosendahl Can you rebase off of master? There were some updates made to the test directory and various test files that should address this error. |
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.
Code looks good now. Test coverage on the module is only 62% which is well below the 80% standard
Coverage is now at 91% for module. |
…rebid#3237) * Updated Bid Adaptor for MultiSized and added Error Call Home * Updated to conform with prebid requirements * added missing this * Added tests for more coverage * simplified error object for edge 15
…rebid#3237) * Updated Bid Adaptor for MultiSized and added Error Call Home * Updated to conform with prebid requirements * added missing this * Added tests for more coverage * simplified error object for edge 15
Type of change
Description of change
Updated Sovrn Adapter to support multisized ads and record errors
For any changes that affect user-facing APIs or example code documented on http://prebid.org, please provide:
Other information