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.

Sign up for GitHub

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

Jump to bottom

Vidazoo Adapter: save and send first request time #11821

Merged
merged 8 commits into from
Jun 26, 2024
Merged

Vidazoo Adapter: save and send first request time #11821

merged 8 commits into from
Jun 26, 2024

Conversation

uditalias
Copy link
Contributor

@uditalias uditalias commented Jun 18, 2024 •
edited
Loading

Type of change

  • Feature

Description of change

Adding first request time param to local storage

@github-actions GitHub Actions
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

Hi, we cannot accept further submissions on your adapter js without an attempt to solve code duplication.

@patmmccann
Copy link
Collaborator

@saar120 you are submitting multiple identical adapters; please adjust to prevent code duplication

image

@uditalias
Copy link
Contributor Author

Hi, we cannot accept further submissions on your adapter js without an attempt to solve code duplication.

Hi @patmmccann
Can you share an example where we can move duplicated code that relates to these specific adapters?

@patmmccann
Copy link
Collaborator

@uditalias please see the links in the github action comment above eg modules/shinezRtbBidAdapter.js has 128 duplicated lines with modules/vidazooBidAdapter.js

@uditalias
Copy link
Contributor Author

@uditalias please see the links in the github action comment above eg modules/shinezRtbBidAdapter.js has 128 duplicated lines with modules/vidazooBidAdapter.js

@patmmccann Yeah, I know, but where should we move the duplicated code? Is there a specific folder in the project for this purpose?

@patmmccann
Copy link
Collaborator

see #8527
https://github.com/prebid/Prebid.js/tree/master/libraries

@uditalias
Copy link
Contributor Author

see #8527 https://github.com/prebid/Prebid.js/tree/master/libraries

Hey @patmmccann
Your example (#8527) relates to the prebid core utils.
The duplicated code presented on this PR is related to specific adapters. Do you know if it is still relevant? Should we create a vidazooUtils folder with our specific functions used by some other adapters?

I just don't want to pollute the public environment of the Libraries folder with some specific adapters library code.

@patmmccann
Copy link
Collaborator

Should we create a vidazooUtils folder with our specific functions used by some other adapters?

Prefect, yes do that

@patmmccann patmmccann self-requested a review June 21, 2024 10:31
@patmmccann patmmccann self-assigned this Jun 21, 2024
@patmmccann
Copy link
Collaborator

#11854 is a great example of a similar effort

@github-actions GitHub Actions
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! 🚀

@github-actions GitHub Actions
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! 🚀

@github-actions GitHub Actions
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! 🚀

@github-actions GitHub Actions
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! 🚀

@github-actions GitHub Actions
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! 🚀

@github-actions GitHub Actions
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! 🚀

@github-actions GitHub Actions
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
patmmccann approved these changes Jun 26, 2024
View reviewed changes
Copy link
Collaborator

@patmmccann patmmccann left a comment

Choose a reason for hiding this comment

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

Vidazoo adapter looks great! An additional pr fixing twist and these other heavily duplicated files is very welcome

@patmmccann
Copy link
Collaborator

Keep up the great work

@patmmccann patmmccann merged commit 0ea2add into prebid:master Jun 26, 2024
4 of 5 checks passed
@patmmccann patmmccann mentioned this pull request Jun 26, 2024
Closed
DecayConstant pushed a commit to mediavine/Prebid.js that referenced this pull request Jul 18, 2024
@uditalias @DecayConstant
Vidazoo Adapter: save and send first request time (prebid#11821)
ebf22b6 
* Vidazoo Adapter: save and send load time

* fix: move shared code into new library

* fix: more utils functions

* fix: moved vidazoo buildRequestData into lib

* fix: moved vidazoo interpretResponse to lib

* fix: moved vidazoo buildRequests to lib

* twistdigital lib fns
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patmmccann patmmccann patmmccann approved these changes

Assignees

@patmmccann patmmccann

Labels
maintenance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@uditalias @patmmccann