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

Documenting the Appnexus test placement #962

Merged
merged 4 commits into from
Aug 1, 2019
Merged

Documenting the Appnexus test placement #962

merged 4 commits into from
Aug 1, 2019

Conversation

hhhjort
Copy link
Collaborator

@hhhjort hhhjort commented Jul 12, 2019

First pass at providing an example for other adapters to follow.

mansinahar
mansinahar previously approved these changes Jul 17, 2019

## Test Request

The following test parameters can be used to verify that Prebid Server is working propely with the
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a typo in this sentence with the word properly. PR LGTM otherwise 👍

Copy link
Contributor

@guscarreon guscarreon left a comment

Choose a reason for hiding this comment

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

LGTM. Just a minor thing and the correct spelling of "properly"


The following test parameters can be used to verify that Prebid Server is working propely with the
Appnexus adapter. This example includes the `imp` object as the test creative must be valid for
serving into the test impression.
Copy link
Contributor

Choose a reason for hiding this comment

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

I find this sentence a bit confusing. Is it missing a coma?

This example includes the `imp` object as the test creative must be valid for serving into the test impression.

Or did we mean:
This example includes the imp object as the test creative and it must be valid for serving into the test impression.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, your sentence says the imp is the test creative, which is not so. We include the imp object so someone using this as a test can create an imp object that the creative can deliver in to.

@mansinahar
Copy link
Contributor

Love the typo fix commit message @hhhjort :p Also, I wouldn't completely disagree with Gus here. I had to read that sentence a couple times too before I could properly understand it. Maybe we can try to simplify it a little bit if possible?

@hhhjort
Copy link
Collaborator Author

hhhjort commented Jul 18, 2019

Yes, it could probably be better. Haven't come up with an improvement yet, I am open to suggestions.

@mansinahar
Copy link
Contributor

Thoughts on something along the lines of:

The following test parameters can be used to verify that Prebid Server is working properly with the 
Appnexus adapter. This example includes an `imp` object with an Appnexus test placement ID and sizes that would match with the test creative.

Copy link
Contributor

@guscarreon guscarreon left a comment

Choose a reason for hiding this comment

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

LGTM

@mansinahar mansinahar merged commit 7c643f8 into master Aug 1, 2019
katsuo5 pushed a commit to flux-dev-team/prebid-server-1 that referenced this pull request Dec 1, 2020
katsuo5 pushed a commit to flux-dev-team/prebid-server-1 that referenced this pull request Dec 2, 2020
katsuo5 pushed a commit to flux-dev-team/prebid-server-1 that referenced this pull request Dec 4, 2020
@SyntaxNode SyntaxNode deleted the appnexus-test branch March 13, 2023 13:53
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.

3 participants