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

Captify RTD Submodule: Initial release #9180

Merged
merged 2 commits into from
Nov 15, 2022

Conversation

captify-osoldatov
Copy link
Contributor

Type of change

  • Feature

Description of change

The RTD submodule enables publishers to use Captify's Live-classification technology via Prebid.js

Contact email of the module maintainer: prebid@captify.tech

@ChrisHuie ChrisHuie requested a review from Fawke November 1, 2022 11:33
@ChrisHuie ChrisHuie changed the title Captify Live-classification Rtd Submodule: Initial release Captify Rtd Submodule: Initial release Nov 1, 2022
@ChrisHuie ChrisHuie changed the title Captify Rtd Submodule: Initial release Captify RTD Submodule: Initial release Nov 1, 2022
Copy link
Contributor

@Fawke Fawke left a comment

Choose a reason for hiding this comment

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

Hi @captify-osoldatov,

Looks good overall, but there are a few things.

  • Missing a Docs PR.
  • I see that you're currently only adding the segment data for Appnexus bidder. Is is possible to make this value available in a more generic place? For example, like the ortb2 object? All bidders can read it then...BTW, Appnexus bid adapter also supports reading from ortb2 keywords now.
  • You would like to add your module to .submodules.json file. Example

@captify-osoldatov
Copy link
Contributor Author

captify-osoldatov commented Nov 14, 2022

Hey @Fawke, thanks for the review!
I've fixed comments in regards of logError and submodules json.

I'm not really sure we want now to create this as ortb2 keywords, we probably can improve this in future releases, if future proof module will work initially as we expect. Currently we tested this together with Xandr (Appnexus) and I don't want to really change the protocol in last moment. I will check this with Xandr for future releases.

Will add documentation PR soon.

Regards,
Oleksandr

@Fawke Fawke removed the needs docs label Nov 15, 2022
@Fawke Fawke merged commit 0813039 into prebid:master Nov 15, 2022
@captify-osoldatov captify-osoldatov deleted the feature/INT-489_captify_rtd_submodule branch November 15, 2022 10:08
JacobKlein26 pushed a commit to nextmillenniummedia/Prebid.js that referenced this pull request Feb 9, 2023
* Captify Live-classification Rtd Submodule

* Fix code-review comments
jorgeluisrocha pushed a commit to jwplayer/Prebid.js that referenced this pull request May 23, 2023
* Captify Live-classification Rtd Submodule

* Fix code-review comments
@patmmccann
Copy link
Collaborator

Hi @captify-osoldatov ; we're not able to carry Captify forward into Prebid 8 without the changes requested above and on #8596 . Since it seems your adapter only handles Xandr and no other adapters, it is better to remove rather than just remove that code block?

@captify-osoldatov
Copy link
Contributor Author

Hi @patmmccann! It's ok to drop it then; For now, I wouldn't be able to provide a quick fix for that. We will better integrate from scratch as soon as we have a new setup and it's tested. Meanwhile, we will still be able to integrate with the earlier Prebid version, so should be fine.
Thanks!

@patmmccann
Copy link
Collaborator

@captify-osoldatov we are happy to provide assistance and point you to demonstrations to get you back integrated quickly!

@captify-osoldatov
Copy link
Contributor Author

Thank you @patmmccann ! Have a nice day)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants