Pubmatic: Making Adslot param optional #3788
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
merging prebid master into fork
merge from prebid master to fork
merge from prebid master to fork
merge from prebid master to fork
Merge prebid master into fork
merge from prebid master
merge latest prebid into fork master
merge latest prebid from master into branch
merge prebid master into fork
merge latest prebid master into branch
updating current branch from master
…ghe/Prebid.js into pm-manasi-moghe-UOE4057_multiformat
Trying to merge Manasi's code
merge prebid master to fork
merge prebid master into MFR branch
Multiformat refactor changes
|
Hello @jsnellbaker , @jaiminpanchal27 |
|
@pm-harshad-mane Have assigned this to myself. I will review soon |
jaiminpanchal27
approved these changes
Apr 30, 2019
| }); | ||
|
|
||
| // commenting following test-case as we are making param adSlot optional |
There was a problem hiding this comment.
Please remove this code if not needed
There was a problem hiding this comment.
Deleted commented test cases
modules/pubmaticBidAdapter.js
Outdated
| @@ -1002,7 +992,7 @@ export const spec = { | |||
| transformBidParams: function (params, isOpenRtb) { | |||
| return utils.convertTypes({ | |||
| 'publisherId': 'string', | |||
| 'adSlot': 'string' | |||
| 'adSlot': 'string' // TODO: do we need to change here as well? | |||
There was a problem hiding this comment.
No this is fine as it is. Type will be converted if it exists
There was a problem hiding this comment.
removed the todo comment
|
@jaiminpanchal27 - ready to merge? |
|
looks to me like the requested changes were made. Merging. |
|
docs PR prebid/prebid.github.io#1299 |
|
Thank you @bretg , @jaiminpanchal27 |
jacekburys-quantcast
pushed a commit
to jacekburys-quantcast/Prebid.js
that referenced
this pull request
May 15, 2019
* changes for multiformat support * added new constant NATIVE_ASSETS in PubMatic adapter * removed NATIVE_ASSET_KEY reference in PubMatic adapter * removed reference of const NATIVE_ASSET_ID from PubMatic adapter * removed reference of const NATIVE_ASSET_DATA_TYPE in PubMatic adapter * using _commonNativeRequestObject in PubMatic adapter to avoid repeating code block * removed new-lines in const declaration * generating NATIVE_ASSET_REVERSE_ID from NATIVE_ASSETS * renamed NATIVE_ASSET_REVERSE_ID to NATIVE_ASSET_ID_TO_KEY_MAP * little modification in _checkParamDataType in PubMatic adapter * a minor improvement * using let instead of var * added NATIVE_ASSET_KEY_TO_ASSET_MAP and combining switch cases * lint update * removed some stale comments * using LOG_WARN_PREFIX * using const UNDEFINED * added a logWarn in catch * using arrow functions * code review changes * making adSlot parameter optional * added a test case for without adSlot config * changes to checkMediaType function * suppress warning of missing mediaTypes.banner for native and video impression * ignore fluid size, if present, in banner impression * fix for ignoring fluid size in banner impression * added relevant comments and test cases for fluid case in banner request * added sample config for multiformat adunit * commented redundant check * marking adSlot param as optional in examples * fixed some lint errors * removed commnented code * removed a trailing white space * removed a todo comment as suggested in review * deleting commented test cases
VideoReach
pushed a commit
to VideoReach/Prebid.js
that referenced
this pull request
Aug 1, 2019
* changes for multiformat support * added new constant NATIVE_ASSETS in PubMatic adapter * removed NATIVE_ASSET_KEY reference in PubMatic adapter * removed reference of const NATIVE_ASSET_ID from PubMatic adapter * removed reference of const NATIVE_ASSET_DATA_TYPE in PubMatic adapter * using _commonNativeRequestObject in PubMatic adapter to avoid repeating code block * removed new-lines in const declaration * generating NATIVE_ASSET_REVERSE_ID from NATIVE_ASSETS * renamed NATIVE_ASSET_REVERSE_ID to NATIVE_ASSET_ID_TO_KEY_MAP * little modification in _checkParamDataType in PubMatic adapter * a minor improvement * using let instead of var * added NATIVE_ASSET_KEY_TO_ASSET_MAP and combining switch cases * lint update * removed some stale comments * using LOG_WARN_PREFIX * using const UNDEFINED * added a logWarn in catch * using arrow functions * code review changes * making adSlot parameter optional * added a test case for without adSlot config * changes to checkMediaType function * suppress warning of missing mediaTypes.banner for native and video impression * ignore fluid size, if present, in banner impression * fix for ignoring fluid size in banner impression * added relevant comments and test cases for fluid case in banner request * added sample config for multiformat adunit * commented redundant check * marking adSlot param as optional in examples * fixed some lint errors * removed commnented code * removed a trailing white space * removed a todo comment as suggested in review * deleting commented test cases
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of change
Description of change
With the addition of default site & ad tag support in at PubMatic Openbid, it's no longer a mandatory requirement to send adslot name parameter.
New behavior: