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

Nexverse bid adapter: Code Refactoring and added suggested changes by Prebid team #12472

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

yogeshverse
Copy link
Contributor

@yogeshverse yogeshverse commented Nov 20, 2024

Type of change

  • Bugfix

  • Feature

  • New bidder adapter

  • Updated bidder adapter

  • Code style update (formatting, local variables)

  • Refactoring (no functional changes, no api changes)

  • Build related changes

  • CI related changes

  • Does this change affect user-facing APIs or examples documented on http://prebid.org?

  • Other

Description of change

Other information

anand-nexverse and others added 9 commits October 7, 2024 06:39
…, and added device and connection type detection
* created utils for all common code
* reused prebid inbuild functions
* created nexverse.html to text nexverse adaptor
* changed test specs based on main js
* created utils for all common code
* reused prebid inbuild functions
* created nexverse.html to text nexverse adaptor
* changed test specs based on main js
* Fixed issues of 1.Category 2.Domain 3.Attributes
* Added Nexverse Demo html for testing with sample bid
* Handled Height and width issue
Copy link

Tread carefully! This PR adds 1 linter warning (possibly disabled through directives):

  • libraries/nexverseUtils/index.js (+1 warning)

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. 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

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

  • libraries/nexverseUtils/index.js (+3 errors)

Copy link

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

  • libraries/nexverseUtils/index.js (+2 errors)

Copy link

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

  • libraries/nexverseUtils/index.js (+2 errors)

@yogeshverse yogeshverse marked this pull request as draft December 3, 2024 06:20
@patmmccann
Copy link
Collaborator

can you rebase with #12297 merging in?

@yogeshverse
Copy link
Contributor Author

can you rebase with #12297 merging in?

Hi @patmmccann , it's already up to date with #12297 , You can ignore this PR as this in draft.

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.

3 participants