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

Feature : Display counter at Adslot level #2802

Merged
merged 1 commit into from
Jul 30, 2018
Merged

Feature : Display counter at Adslot level #2802

merged 1 commit into from
Jul 30, 2018

Conversation

vedantseta
Copy link
Contributor

Type of change

  • Feature

Description of change

As discussed in #2295 thread, some bidders will be interested in getting the Display Counter which is added in this PR.

buildRequests of an adapter will be called with an key displayCount at adSlot level.

This PR also contains Media.net adapter integration with this feature

@vedantseta vedantseta changed the title Display counter at Adslot level Feature : Display counter at Adslot level Jul 3, 2018
@vedantseta
Copy link
Contributor Author

Hi @idettman @mike-chowla , can you please check this PR?

derdeka
derdeka suggested changes Jul 25, 2018
View reviewed changes
Copy link
Contributor

@derdeka derdeka left a comment

Choose a reason for hiding this comment

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

Do you think its a good idea to reintroduce $$PREBID_GLOBAL$ at so many places? Isn't there an alternative for that?

@vedantseta
Copy link
Contributor Author

Hi @derdeka ,
Thanks for having a look. Not sure if there is other way around.
Happy to include if you suggest better alternative.

Thanks.

@idettman idettman merged commit 54f23fa into prebid:master Jul 30, 2018
@mkendall07
Copy link
Member

This doesn't need to be exposed on the public global

@mkendall07
Copy link
Member

@vedantseta
Can you explain why this needs to be on $$PREBID_GLOBAL$$ ? My understanding is that you would introduce a new private global internal to prebid.js. We need to revert this as it has no reason to be on the public API.

@vedantseta
Copy link
Contributor Author

Agreed @mkendall07, this should not be public. We will make those changes.

mkendall07 added a commit that referenced this pull request Jul 31, 2018
@mkendall07
Revert "added display count global object to track number of times re…
bccffa0 
…questBids has been called per ad-unit (#2802)"

This reverts commit 54f23fa.
@mkendall07 mkendall07 mentioned this pull request Jul 31, 2018
Merged
@mkendall07
Copy link
Member

thanks. Will revert this for now. Feel free to open a new PR.

jsnellbaker pushed a commit that referenced this pull request Jul 31, 2018
@mkendall07 @jsnellbaker
Revert "Feature : Display counter at Adslot level " (#2912)
386fbac 
* Revert "Add onBidWon method to bid adapter spec (#2786)"

This reverts commit 838ccee.

* Revert "added display count global object to track number of times requestBids has been called per ad-unit (#2802)"

This reverts commit 54f23fa.
@vedantseta vedantseta mentioned this pull request Aug 7, 2018
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derdeka derdeka derdeka requested changes

@idettman idettman idettman approved these changes

Assignees

@idettman idettman

Labels
needs review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@vedantseta @mkendall07 @idettman @derdeka @mike-chowla @binoy-chitale