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

DSPx Bid Adapter: add ortb2 content, topics support #11941

Merged

Conversation

onlsol
Copy link
Contributor

@onlsol onlsol commented Jul 8, 2024

DSPx Bid Adapter: add ortb2 content, topics support

Type of change

  • Feature

Description of change

Improved the adapters support for ortb content, userids, topics.

DSPx Bid Adapter: add ortb2 content, topics support
Copy link

github-actions bot commented Jul 8, 2024

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. We hate that we have to mention this, however, commits designed to hide from this utility by renaming variables or reordering an object are poor conduct. We will not look upon them kindly! Keep up the great work! 🚀

Copy link

github-actions bot commented Jul 8, 2024

Tread carefully! This PR adds 6 linter errors (possibly disabled through directives):

  • test/spec/modules/dspxBidAdapter_spec.js (+6 errors)

Copy link

github-actions bot commented Jul 8, 2024

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. We hate that we have to mention this, however, commits designed to hide from this utility by renaming variables or reordering an object are poor conduct. We will not look upon them kindly! Keep up the great work! 🚀

@patmmccann
Copy link
Collaborator

Hi! it appears DSPx and STV are very closely mirrored. Please see #11879 and #8527 and #11854 and #11877 for examples on fixing. Thanks!

@onlsol
Copy link
Contributor Author

onlsol commented Jul 8, 2024

@patmmccann Hi, that's right, the stvBidAdapter has several similarities in requirements, code constructs, conditions and code comments as dspxBidAdapter.
As far as I see, none of them is related to the current PR's changes or commits.
Would it be possible to proceed with this PR as it is (type of change "feature")? If you agree, subsequently, in a separate PR, we could work on the maintenance/refactoring and move re-usable parts to bidderUtils.

@patmmccann
Copy link
Collaborator

Yes but please make sure that is your next pr; we are in the midst of an initiative to identify and consolidate duplicated code :)

@patmmccann patmmccann self-assigned this Jul 8, 2024
@patmmccann patmmccann merged commit ba82380 into prebid:master Jul 8, 2024
3 of 5 checks passed
@patmmccann
Copy link
Collaborator

btw feature is typically reserved for new modules, support for new media types, and changes to the core

@onlsol
Copy link
Contributor Author

onlsol commented Jul 8, 2024

Understood, thanks for clarification.

DecayConstant pushed a commit to mediavine/Prebid.js that referenced this pull request Jul 18, 2024
* DSPx Bid Adapter: add ortb2 content, topics support

DSPx Bid Adapter: add ortb2 content, topics support

* DSPx Bid Adapter: add ortb2 content, topics support

---------

Co-authored-by: avj <[email protected]>
mefjush pushed a commit to adhese/Prebid.js that referenced this pull request Jul 19, 2024
* DSPx Bid Adapter: add ortb2 content, topics support

DSPx Bid Adapter: add ortb2 content, topics support

* DSPx Bid Adapter: add ortb2 content, topics support

---------

Co-authored-by: avj <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants