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

Currency Module: always adding originalCpm and originalCurrency to bid object #3856

Merged
merged 2 commits into from
Jun 3, 2019

Conversation

pm-harshad-mane
Copy link
Contributor

Type of change

  • Code style update (formatting, local variables)

Description of change

#3855
Always add originalCpm and originalCurrency to bid object when the currency module is enabled.

@pm-harshad-mane
Copy link
Contributor Author

Hello @jsnellbaker ,
Could you please review?

@pm-harshad-mane
Copy link
Contributor Author

Hello @jsnellbaker ,
A gentle reminder :)
Could you please review the change?

@jaiminpanchal27
Copy link
Collaborator

@pm-harshad-mane I am not 100% sure here but undefined shouldn't be problem here. If it is undefined it clearly means that bidder responded in same currency and there is no need to store same data in duplicate property.

I guess analytics adapters or listeners can add a check for these properties and can easily figure this out.

@jaiminpanchal27 jaiminpanchal27 requested a review from idettman May 29, 2019 20:31
@pm-harshad-mane
Copy link
Contributor Author

@jaiminpanchal27 analytics adapters and listeners can add this un-documented check but I think it is better if currency module always sets these two fields so that the listeners and analytics adapters will not need this special handling and it will also make the bid-object have consistent fields.

@idettman has approved the change does that mean we can merge the change in master?

@jsnellbaker jsnellbaker merged commit ac65812 into prebid:master Jun 3, 2019
@pm-harshad-mane
Copy link
Contributor Author

Thank you @idettman , @jaiminpanchal27 , @jsnellbaker :)

ghost pushed a commit to devunrulymedia/Prebid.js that referenced this pull request Jun 7, 2019
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants