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.

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

few changes #681

Closed
wants to merge 7 commits into from
Closed

few changes #681

wants to merge 7 commits into from

Conversation

sekindo
Copy link
Contributor

@sekindo sekindo commented Oct 6, 2016

Type of change

  • Bugfix
  1. remove of setting the bid.adId
  2. using https only when needed

@mkendall07
Copy link
Member

@sekindo
Why are you removing bid.adId ? Please see #509 - we want to match bid ids to bid requests. Thanks

@sekindo
Copy link
Contributor Author

sekindo commented Oct 9, 2016

We removed the bid.adId since we see that no other adapter is setting this parameter.
as far as we understand, this parameter is being set by the prebid library and there is no need to set it in the adapter.

@protonate
Copy link
Collaborator

We are recommending that adapters use the same ID for the bid response as was generated in the bid request. You can achieve this by passing the bidRequest object as the second argument to bidfactory.createBid for both a valid bid response and no bid scenarios. See https://github.com/prebid/Prebid.js/blob/master/src/bidfactory.js#L18

@sekindo
Copy link
Contributor Author

sekindo commented Oct 19, 2016

added bidRequest object as the second argument to bidfactory.createBid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants