Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Serverbid alias #1560

Merged
merged 4 commits into from
Sep 18, 2017
Merged

Serverbid alias #1560

merged 4 commits into from
Sep 18, 2017

Conversation

jgrimes
Copy link
Contributor

@jgrimes jgrimes commented Sep 5, 2017

Type of change

  • Feature

Description of change

Added extra configuration for Serverbid adapter and two adapter alises, connectad and oft-media.

@protonate protonate requested review from matthewlane and jaiminpanchal27 and removed request for matthewlane September 7, 2017 19:08
jaiminpanchal27
jaiminpanchal27 reviewed Sep 11, 2017
View reviewed changes
modules/serverbidBidAdapter.js
params.bids.length &&
CONFIG[params.bidderCode]) {
const config = CONFIG[params.bidderCode];
config.request = window[params.bidderCode.toUpperCase() + '_CONFIG'];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jgrimes What is the purpose of adding this line. This will be set to undefined, i guess your library will set window[params.bidderCode.toUpperCase() + '_CONFIG'];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaiminpanchal27 It is to allow people using our adapter (or aliases) to add arbitrary additional configuration/information on the requests being made to us.

jaiminpanchal27
jaiminpanchal27 approved these changes Sep 12, 2017
View reviewed changes
Copy link
Collaborator

@jaiminpanchal27 jaiminpanchal27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

mkendall07
mkendall07 reviewed Sep 13, 2017
View reviewed changes
modules/serverbidBidAdapter.js Outdated
adaptermanager.registerBidAdapter(new ServerBidAdapter(), 'serverbid');
adaptermanager.aliasBidAdapter('serverbid', 'connectad');
adaptermanager.aliasBidAdapter('serverbid', 'oft-media');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oftmedia is already a bid adapter alias of AppNexus. Should that be removed? It's confusing.

@mkendall07
Copy link
Member

@jgrimes
can you explain what oftmedia is? It's already an alias fro AppNexus.

@jgrimes jgrimes force-pushed the serverbid-alias branch 2 times, most recently from 8db768c to 1ece02d Compare September 18, 2017 16:10
micha and others added 4 commits September 18, 2017 13:39
@micha @jgrimes
Connectad alias, request-level configuration, new ad sizes
68a4369 
- Add 3 new ad sizes.
- Create connectad alias.
- Refactor to support whitelabel URLs by alias.
- Add support for window.<ADAPTER>_CONFIG object for request level
  params.
@jgrimes
Add oft-media alias for serverbid
f112c42
@jgrimes
serverbidBidAdapter:Fixed formatting issues
0eebe2b
@jgrimes
Serverbid: Updated smartsync urls, removed oft-media alias
ed01ff4
@jgrimes
Copy link
Contributor Author

jgrimes commented Sep 18, 2017

@mkendall07 we've removed the oft-media alias for now but will want to add it in the future. The "owner" of the other oftmedia alias wants this one as well.
@jaiminpanchal27 I made an update to this code to fix our adapter alias and adjust some URLs. If you can give it another review it would be much appreciated.

@jaiminpanchal27 jaiminpanchal27 merged commit 5b99aaa into prebid:master Sep 18, 2017
outoftime pushed a commit to Genius/Prebid.js that referenced this pull request Sep 18, 2017
Merge branch 'master' of https://github.com/prebid/Prebid.js into pre…
77678fc 
…built

* 'master' of https://github.com/prebid/Prebid.js: (46 commits)
  Serverbid alias (prebid#1560)
  Add user sync to process for approving adapter PRs (prebid#1457)
  fix travis build (prebid#1595)
  Rubicon project improvement/user sync (prebid#1549)
  Adding Orbitsoft adapter (prebid#1378)
  Fix renderer test for new validation rule (prebid#1592)
  Allow SET_TARGETING to be used in AnalyticsAdapter (prebid#1577)
  Add support for video stream context (prebid#1483)
  Invalidate bid if matching bid request not found (prebid#1575)
  allow adapters to set default adserverTargeting for specific bid (prebid#1568)
  Custom granularity precision should honor 0 if it is passed in closes prebid#1479 (prebid#1591)
  BaseAdapter for the Prebid 0.x -> 1.x transition  (prebid#1494)
  Add a version to the Criteo adapter (prebid#1573)
  Allow bundling from node.js or with new gulp task bundle-to-stdout  (prebid#1570)
  Add url.parse option to not decode the whole URL (prebid#1480)
  Tremor Video Bid Adapter (prebid#1552)
  Yieldmo bid adapter (prebid#1415)
  Switch `gulp docs` to build its output using documentation.js (prebid#1545)
  Increment pre version
  Prebid 0.28.0 Release
  ...
jbAdyoulike pushed a commit to jbAdyoulike/Prebid.js that referenced this pull request Sep 21, 2017
@jgrimes @jbAdyoulike
Serverbid alias (prebid#1560)
9ddbff0 
* Connectad alias, request-level configuration, new ad sizes

- Add 3 new ad sizes.
- Create connectad alias.
- Refactor to support whitelabel URLs by alias.
- Add support for window.<ADAPTER>_CONFIG object for request level
  params.

* Add oft-media alias for serverbid

* serverbidBidAdapter:Fixed formatting issues

* Serverbid: Updated smartsync urls, removed oft-media alias
outoftime pushed a commit to Genius/Prebid.js that referenced this pull request Sep 21, 2017
Merge tag '0.29.0' of https://github.com/prebid/Prebid.js into prebuilt
3fdf977 
* tag '0.29.0' of https://github.com/prebid/Prebid.js: (29 commits)
  Prebid 0.29.0 Release
  Fix for not syncing bidders.  (prebid#1598)
  fix amp example pages (prebid#1597)
  closes prebid#1298 (prebid#1583)
  Fixed the broken tests. (prebid#1602)
  Rubicon Bidder Factory (prebid#1587)
  Trustx adapter (prebid#1488)
  Add nurl to markup (prebid#1601)
  Pass bidRequest to createBid (prebid#1600)
  Add Kumma adapter (prebid#1512)
  Serverbid alias (prebid#1560)
  Add user sync to process for approving adapter PRs (prebid#1457)
  fix travis build (prebid#1595)
  Rubicon project improvement/user sync (prebid#1549)
  Adding Orbitsoft adapter (prebid#1378)
  Fix renderer test for new validation rule (prebid#1592)
  Allow SET_TARGETING to be used in AnalyticsAdapter (prebid#1577)
  Add support for video stream context (prebid#1483)
  Invalidate bid if matching bid request not found (prebid#1575)
  allow adapters to set default adserverTargeting for specific bid (prebid#1568)
  ...
@rmloveland rmloveland mentioned this pull request Oct 16, 2017
Merged
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
@jgrimes @dluxemburg
Serverbid alias (prebid#1560)
b639a0f 
* Connectad alias, request-level configuration, new ad sizes

- Add 3 new ad sizes.
- Create connectad alias.
- Refactor to support whitelabel URLs by alias.
- Add support for window.<ADAPTER>_CONFIG object for request level
  params.

* Add oft-media alias for serverbid

* serverbidBidAdapter:Fixed formatting issues

* Serverbid: Updated smartsync urls, removed oft-media alias
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaiminpanchal27 jaiminpanchal27 jaiminpanchal27 approved these changes

@mkendall07 mkendall07 mkendall07 approved these changes

Assignees

@jaiminpanchal27 jaiminpanchal27

Labels
LGTM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jgrimes @mkendall07 @jaiminpanchal27 @micha