Skip to content

New Adapter: Impactify#2004

Merged
SyntaxNode merged 9 commits intoprebid:masterfrom
thomasdseao:feature/impactify
Oct 5, 2021
Merged

New Adapter: Impactify#2004
SyntaxNode merged 9 commits intoprebid:masterfrom
thomasdseao:feature/impactify

Conversation

@thomasdseao
Copy link
Contributor

Impactify PBS Adapter.

Docs PR : prebid/prebid.github.io#3273

@SyntaxNode SyntaxNode changed the title Impactify Adapter New Adapter: Impactify Sep 17, 2021
@SyntaxNode SyntaxNode self-assigned this Sep 17, 2021
Copy link
Contributor

@SyntaxNode SyntaxNode left a comment

Choose a reason for hiding this comment

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

Thank you for the quick update from my feedback. In this pass I'm asking for improved defensive programming (don't ignore errors) and an increase in json test coverage.

@MartianTribe
Copy link

Related docs PR: prebid/prebid.github.io#3273

Copy link
Contributor

@SyntaxNode SyntaxNode left a comment

Choose a reason for hiding this comment

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

Looks good. Just a few more test comments. I think you'll need to add a new http_204.json test case and for the headers you can expand the existing headers.json IMHO.

Copy link
Contributor

@SyntaxNode SyntaxNode left a comment

Choose a reason for hiding this comment

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

Looks good to me now, except for the small marsmedia comment. Thank you for making the requested changes and strengthening the test coverage. A second reviewer will take a look soon.

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.

One last nitpick.

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

@SyntaxNode SyntaxNode merged commit 078ce00 into prebid:master Oct 5, 2021
jizeyopera pushed a commit to operaads/prebid-server that referenced this pull request Oct 13, 2021
Co-authored-by: Thomas De Stefano <thomas.destefano@impactify.io>
jorgeluisrocha pushed a commit to jwplayer/prebid-server that referenced this pull request Sep 28, 2022
Co-authored-by: Thomas De Stefano <thomas.destefano@impactify.io>
shunj-nb pushed a commit to ParticleMedia/prebid-server that referenced this pull request Nov 8, 2022
Co-authored-by: Thomas De Stefano <thomas.destefano@impactify.io>
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.

6 participants

Comments