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

ORTB 2.6: Harden SetImps to cover larger changes #4004

Merged
merged 3 commits into from
Oct 22, 2024
Merged

Conversation

hhhjort
Copy link
Collaborator

@hhhjort hhhjort commented Oct 21, 2024

No description provided.

@bsardo bsardo changed the title Harden SetImps to cover larger changes. ORTB 2.6: Harden SetImps to cover larger changes Oct 21, 2024
@bsardo
Copy link
Collaborator

bsardo commented Oct 21, 2024

LGTM but should we add a unit test for SetImp that ensures the same number of elements appear in Imp and the imp wrappers and that they reference the same addresses via assert.Same?

openrtb_ext/request_wrapper.go Outdated Show resolved Hide resolved
openrtb_ext/request_wrapper.go Outdated Show resolved Hide resolved
openrtb_ext/request_wrapper_test.go Outdated Show resolved Hide resolved
@bsardo bsardo self-requested a review October 21, 2024 20:33
@bsardo bsardo merged commit d6b07ab into ortb26 Oct 22, 2024
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.

4 participants