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

rename Adapter from liftoff to vungle #5396

Merged
merged 9 commits into from
Jun 27, 2024
Merged

Conversation

Vungle-GordonTian
Copy link
Contributor

@Vungle-GordonTian Vungle-GordonTian commented Jun 5, 2024

🏷 Type of documentation

  • new bid adapter
  • update bid adapter
  • new feature
  • text edit only (wording, typos)
  • bugfix (code examples)
  • new examples

📋 Checklist

Copy link

netlify bot commented Jun 5, 2024

Deploy Preview for prebid-docs-preview ready!

Name Link
🔨 Latest commit d0c618d
🔍 Latest deploy log https://app.netlify.com/sites/prebid-docs-preview/deploys/66751339ea3c140008362f0e
😎 Deploy Preview https://deploy-preview-5396--prebid-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@bretg bretg left a comment

Choose a reason for hiding this comment

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

We don't allow abrupt renames like this without a transition period. Liftoff can become an alias of vungle and then at some point in a major release liftoff can be removed

  1. update the code so the vungle adapter lists liftoff as an alias.
  2. leave liftoff documentation in place, create vungle docs

@Vungle-GordonTian
Copy link
Contributor Author

We don't allow abrupt renames like this without a transition period. Liftoff can become an alias of vungle and then at some point in a major release liftoff can be removed

  1. update the code so the vungle adapter lists liftoff as an alias.
  2. leave liftoff documentation in place, create vungle docs

@bretg I'm an Engineer from Liftoff, and we are integrating with @PBDMSFT on this adapter for a while.
I'm sure that no other publishers outside of MCG are calling our Prebid-Server adapter today, and I also confirmed with our business side colleagues on this. So it is safe to do the rename, could you please review this PR again?

@bretg
Copy link
Contributor

bretg commented Jun 18, 2024

@Vungle-GordonTian - I'm not sure how you can be so certain that no one else has enabled liftoff. How about a compromise:

  • you can rename the PBS adapter (since it was already approved). I would really have preferred to not change the original name, but will accept that cat's out of the bag.
  • you'll add 'liftoff' as an alias to the PBS adapter
  • you'll add liftoff.md to this docs PR with a new metadata entry of aliasCode: vungle

This way if someone complains to me that we broke their configuration by allowing this rename to take place, we can apologize and give them instructions to replace their YAML file.

@Vungle-GordonTian
Copy link
Contributor Author

Vungle-GordonTian commented Jun 19, 2024

@Vungle-GordonTian - I'm not sure how you can be so certain that no one else has enabled liftoff. How about a compromise:

  • you can rename the PBS adapter (since it was already approved). I would really have preferred to not change the original name, but will accept that cat's out of the bag.
  • you'll add 'liftoff' as an alias to the PBS adapter
  • you'll add liftoff.md to this docs PR with a new metadata entry of aliasCode: vungle

This way if someone complains to me that we broke their configuration by allowing this rename to take place, we can apologize and give them instructions to replace their YAML file.

@PBDMSFT Will Xandr work as expected if we use this alias way that @bretg mentioned?
If you still get liftoff as adapter name this way, then I think this alias method will not work for our case.

@PBDMSFT
Copy link

PBDMSFT commented Jun 19, 2024

@Vungle-GordonTian We've already made changes on our side to accommodate "vungle". If you add an alias for "liftoff", the adapter name we originally had, it won't hurt anything. Please keep the "vungle" adapter in place though as that change was already released in Prebid Server and updated on our end.

@Vungle-GordonTian
Copy link
Contributor Author

@Vungle-GordonTian We've already made changes on our side to accommodate "vungle". If you add an alias for "liftoff", the adapter name we originally had, it won't hurt anything. Please keep the "vungle" adapter in place though as that change was already released in Prebid Server and updated on our end.

Get it. Then we can go on with our integrating test. I will keep vungle adapter in place. I will add alias as Bretg mentioned simutaneously.

@muuki88 muuki88 requested a review from bretg June 27, 2024 14:28
@muuki88 muuki88 merged commit 15a5845 into prebid:master Jun 27, 2024
5 checks passed
BIGOAds pushed a commit to BIGOAds/prebid.github.io that referenced this pull request Jul 5, 2024
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