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

Preciso Bid Adapter : update on valid request checks #11745

Closed
wants to merge 0 commits into from

Conversation

NikhilGopalChennissery
Copy link
Contributor

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

@patmmccann
Copy link
Collaborator

patmmccann commented Jun 10, 2024

It appears your duplication fixes were tricks to avoid the report. Please actually fix by creating libraries and importing common code into your modules and others flagged

}
if (bidderRequest.gdprConsent) {
request.gdpr = bidderRequest.gdprConsent
req.gdpr = bidderRequest.gdprConsent
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are many tricks to fool the duplication detector in this pr. Remove them.

@NikhilGopalChennissery
Copy link
Contributor Author

NikhilGopalChennissery commented Jun 11, 2024

Hi @patmmccann
Sorry for the changes we have made. We are beginners on the Prebid platform.
We created our Adapter by referring to the documentation on prebid.org and examining other bidAdapters' code as well. We have noticed that some of codes is common in some other adapters as well.

I am ready to modify the code as you suggested. I would like to clarify a few points:

We are accessing storage manager from the BidAdapter itself. Is that okay?
For building Bid requests and Bid responses, should I create a common library function and use that library in our bid adapter.?

Please correct me if I am wrong.

Thank you.

@patmmccann
Copy link
Collaborator

You may access storage manager, however you will not have access by default, publishers must grant access.

Yes, importing functions from common libraries should reduce code duplication. Your particular adapter looks similar to at least one adapter maintained by teqblaze, who I believe has that effort underway. You may wish to coordinate with them or look at their patterns

MaksymTeqBlaze@0249eb7

@NikhilGopalChennissery
Copy link
Contributor Author

Hi @patmmccann ,

Thanks for your update. I hope this will help us to remove the code duplications

We will work on this and update the PR ASAP.

Thank you!

@NikhilGopalChennissery
Copy link
Contributor Author

Hi @patmmccann ,

I have created a new PR ( #11868 ) with our latest changes to remove the code duplication.

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