Dailymotion Bid Adaptor: add support for user syncs & new fields #25
Conversation
kvnsw
requested review from
rumesh,
florianorpeliere,
jeremy-le-dev and
sebmil-daily
|
|
||
| expect(reqData.request.mediaTypes.video).to.eql(bidRequestData[0].mediaTypes.video); |
There was a problem hiding this comment.
isn't it better to have the unit test covering each attribute than the full object? then when one attribute is failing we will have the exact attribute where an issue is occuring instead of the whole object?
|
|
||
| expect(reqData.request.mediaTypes.video).to.eql(bidRequestData[0].mediaTypes.video); | ||
|
|
There was a problem hiding this comment.
isn't it better to have the unit test covering each attribute than the full object? then when one attribute is failing we will have the exact attribute where an issue is occuring instead of the whole object?
There was a problem hiding this comment.
Oh actually it shouldn't be an issue, the failure will produce the diff showing which attribute has an issue (actual vs expected)
kvnsw
force-pushed
the
feat/dailymotion-support-usersync
branch
from
ca51a15 to
3948851
Compare
kvnsw
force-pushed
the
feat/dailymotion-support-usersync
branch
from
3948851 to
16c1e56
Compare
@rumesh, to answer your comment, we are moving forward with grouped contributions to have Prebid org review a single PR for all features (no multi-assignments of reviewers, and everything in a single process). We'll ask them for feedback if they would prefer a one PR per feature approach. |
Type of change
Description of change
plcmt&playbackmethod