Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
New Adapter: Pixfuture #4117
base: master
Are you sure you want to change the base?
New Adapter: Pixfuture #4117
Changes from 30 commits
93e6573
458fad1
d67a079
a9fe81b
606b185
061b818
d0bc2e1
eec17ff
cb8e3a7
bba6eed
22df0f8
3f33089
a22b19e
993f157
284c847
9e470a0
3b4ca26
1df8f71
6a59158
2ba1a64
ea7d94e
8dac147
078c276
0b62c21
7bdabf4
3f219c9
bb9b65d
7a3352f
3f13c25
a407b22
3457b8c
f1b859c
025a574
eece8b6
33fbff5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can delete all of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please follow the guidance in the Bidder Parameter Tests developer docs section on how to test your bidder params, which in this case, appears to just be
pix_id
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This validation check is not needed since you've only declared support for banner and video in your YAML file and PBS core will only call your adapter if it contains media types you declare support for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated as per your suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation check still exists. Please delete as previously discussed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit tests are considered nice to haves. The JSON test framework is required to achieve code coverage wherever possible. Please see the developer docs Test Your Adapter section on how to test your adapter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test JSONs have been reviewed and edited.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test stuff has been reviewed and recreated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of your JSON tests are being executed because you're not calling
RunJSONBidderTest
as described in the Test Your Adapter section of the developer docs. Please add aTestJsonSamples
test as described in the docs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please delete this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file still exists. Please delete.