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

Adagio Rtd Provider: add placementSource param #11779

Conversation

osazos
Copy link
Collaborator

@osazos osazos commented Jun 12, 2024

Type of change

  • Feature
  • Updated RTD provider

Description of change

  • Add a config param to define a source to programmatically set the value of ortb2Imp.ext.data.placement signal
  • Remove the behavior that was to always fallback on the ad-unit code value, but only when reading the Adagio params in adUnits[].bids[]

Other information

Updated doc: prebid/prebid.github.io#5415

@osazos osazos changed the title AdagioRtdProvider: add placementSource param Adagio Rtd Provider: add placementSource param Jun 12, 2024
Copy link

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

  • modules/adagioRtdProvider.js has 8 duplicated lines with modules/adagioRtdProvider.js
  • modules/adagioRtdProvider.js has 19 duplicated lines with modules/bliinkBidAdapter.js

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. Keep up the great work! 🚀

@osazos osazos force-pushed the adagio-fix/rely-on-params-to-autodetect-placement branch from 360e20d to 73536a6 Compare June 17, 2024 15:37
Copy link

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. Keep up the great work! 🚀

@patmmccann
Copy link
Collaborator

Hey @osazos I merged your other two pr's, can you clean up the last duplication issue on this one by moving

const _ADAGIO = (function() {
into /libraries/adagioUtils/ ?

@osazos osazos force-pushed the adagio-fix/rely-on-params-to-autodetect-placement branch from 73536a6 to e6237ce Compare June 21, 2024 11:53
@ChrisHuie ChrisHuie self-assigned this Jun 21, 2024
@ChrisHuie ChrisHuie merged commit b92bff3 into prebid:master Jun 21, 2024
5 checks passed
@osazos osazos deleted the adagio-fix/rely-on-params-to-autodetect-placement branch June 21, 2024 12:50
DecayConstant pushed a commit to mediavine/Prebid.js that referenced this pull request Jul 18, 2024
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.

3 participants