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

Add bid adapter for Yearxero; #2948

Merged
merged 2 commits into from
Feb 6, 2024

Conversation

BogdanKosarev
Copy link
Contributor

@BogdanKosarev BogdanKosarev commented Feb 1, 2024

Adding a new bid adapter for Yearxero.

@BogdanKosarev BogdanKosarev changed the title added bid adapter for Yearxero; Add bid adapter for Yearxero; Feb 1, 2024
import java.util.List;
import java.util.Objects;

public class YearxeroBidder implements Bidder<BidRequest> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

as I can judge this bidder can be implemented as an alias of the generic bidder since the logic is straightforward

so please, check the generic.yaml and find there examples of the aliases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the logic of our adapter is straightforward right now but we planning to add some updates in future. Also, I found few adapters which are also simple but don't declared as aliases.
Is aliasing of such kind of adapters obligatory?

Copy link
Collaborator

@AntoxaAntoxic AntoxaAntoxic Feb 6, 2024

Choose a reason for hiding this comment

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

I believe the reason why we have adapters that are great candidates for being aliases, but they are still not, because the "plans to add some updates in future" has never been implemented. That's why we have an ongoing task to convert them to the aliases.

Do you plan on using the adapter before adding the updates you mentioned?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you plan on using the adapter before adding the updates you mentioned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we would like to start as soon as possible and we have a list of improvements for our bidder in our backlog for implementing. We are going to work in parallel on both sides: on adapter and bidder.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alright, if you have further plans on updating the bidder, let it be a separate one


@AllArgsConstructor(staticName = "of")
@Value
public class ExtImpYearxero {
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like this class isn't used anywhere, so it could be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this class is not used directly anywhere. But I thought that we need to declare such classes for custom extensions in order to make using of our adapter more clear, am I right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, it's not necessary at all while the adapter just passes through everything as-is. So in order to have the code clean, I suggest removing parts that are not used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I removed the file.

@SerhiiNahornyi SerhiiNahornyi merged commit 3d33c06 into prebid:master Feb 6, 2024
3 checks passed
@BogdanKosarev BogdanKosarev deleted the add-yearxero-adapter branch February 6, 2024 19:41
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