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

Browsi Viewability Module #1494

Merged
merged 5 commits into from
Nov 7, 2019
Merged

Browsi Viewability Module #1494

merged 5 commits into from
Nov 7, 2019

Conversation

omerDotan
Copy link
Contributor

Docs for Real-time data module & browsi real-time data Provider

pbjs.setConfig({
"realTimeData": {
"auctionDelay": 1000,
dataProviders[{
Copy link
Contributor

Choose a reason for hiding this comment

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

Need a semi colon after dataProviders

dataProviders[{
"name": "browsi",
"params": {
"url": "testUrl.com",
Copy link
Contributor

Choose a reason for hiding this comment

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

If you use this example, you get an error:

ERROR: missing params for Browsi provider

Maybe we should use the example from browsiRtdProvider.md

@bretg
Copy link
Contributor

bretg commented Oct 31, 2019

fixed conflicts

@Asafsham
Copy link

@bretg Looks good!
@omerBrowsi Making sure you noticed it.

@bretg bretg changed the title Real time data module & browsi Rtd Provider Browsi Viewability Module Oct 31, 2019
Copy link
Contributor

@jeanstemp jeanstemp left a comment

Choose a reason for hiding this comment

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

Suggested edits for clarity

dev-docs/modules/browsiRtdProvider.md Outdated Show resolved Hide resolved
dev-docs/modules/browsiRtdProvider.md Outdated Show resolved Hide resolved
dev-docs/modules/browsiRtdProvider.md Outdated Show resolved Hide resolved
dev-docs/modules/browsiRtdProvider.md Outdated Show resolved Hide resolved
dev-docs/modules/browsiRtdProvider.md Outdated Show resolved Hide resolved
dev-docs/modules/browsiRtdProvider.md Outdated Show resolved Hide resolved
dev-docs/modules/browsiRtdProvider.md Outdated Show resolved Hide resolved
dev-docs/modules/browsiRtdProvider.md Outdated Show resolved Hide resolved
dev-docs/modules/browsiRtdProvider.md Outdated Show resolved Hide resolved
Covered all of Jean's comments.
@bretg bretg requested a review from jeanstemp November 6, 2019 22:39
@jeanstemp jeanstemp merged commit 161c90f into prebid:master Nov 7, 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.

5 participants