Skip to content
This repository has been archived by the owner on Oct 6, 2022. It is now read-only.

OpenRTB 2.6 #48

Merged
merged 62 commits into from
May 19, 2022
Merged

OpenRTB 2.6 #48

merged 62 commits into from
May 19, 2022

Conversation

SyntaxNode
Copy link
Contributor

Implements #47. Includes all changes in OpenRTB 2.6 and parity changes in AdCOM 1.0.

adcom1/api_framework.go Show resolved Hide resolved
adcom1/category_taxonomy.go Show resolved Hide resolved
Protocols []Protocol `json:"protocols,omitempty"`
// Array of supported audio protocols. Refer to List: Creative
// Subtypes - Audio/Video in AdCOM 1.0.
Protocols []int8 `json:"protocols,omitempty"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All enumeration lists (formerly section 5) have been removed from OpenRTB 2.6 in favor of the definitions in AdCOM 1.0. I disassociated the enums for now, but do you think it's better to directly reference the /adcom1 package here? .. or copy the AdCOM enums to /openrtb2?

Copy link
Owner

Choose a reason for hiding this comment

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

Just commented above, I think it will be better to import adcom1 in openrtb2 since it relies so heavily on adcom now.

I'll get back to it once done with other bits.

@mxmCherry
Copy link
Owner

I expected 2.6 to result in fewer changes, that's quite a big PR. Thanks a lot! Also preparing next major release (boring service work) - much appreciated as well.

I left a couple of 👍 comments, and only quickly eyeballed the diff - so far seems good.

I'll do a proper/detailed review (comparing to 2.6 spec if anything's missed + what to do with Audio.Protocols type) over this weekend, so expecting a release (v16) somewhere around next Mon-Wed.

@SyntaxNode
Copy link
Contributor Author

I expected 2.6 to result in fewer changes, that's quite a big PR.

Agreed. There were more changes than I expected.

Thanks a lot! Also preparing next major release (boring service work) - much appreciated as well.

Happy to help. :)

I'll do a proper/detailed review (comparing to 2.6 spec if anything's missed + what to do with Audio.Protocols type) over this weekend, so expecting a release (v16) somewhere around next Mon-Wed.

Cool. Thank you.

adcom1/agent_type.go Outdated Show resolved Hide resolved
openrtb2/app.go Outdated Show resolved Hide resolved
openrtb2/audio.go Outdated Show resolved Hide resolved
openrtb2/audio.go Outdated Show resolved Hide resolved
openrtb2/audio.go Outdated Show resolved Hide resolved
openrtb2/audio.go Outdated Show resolved Hide resolved
Delivery []ContentDeliveryMethod `json:"delivery,omitempty"`
// none specified, assume all are supported. Refer to List: Delivery
// Methods in AdCOM 1.0.
Delivery []int8 `json:"delivery,omitempty"`
Copy link
Owner

Choose a reason for hiding this comment

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

TODO: maybe import adcom1 here

openrtb2/audio.go Outdated Show resolved Hide resolved
openrtb2/audio.go Outdated Show resolved Hide resolved
openrtb2/content.go Outdated Show resolved Hide resolved
@SyntaxNode
Copy link
Contributor Author

openrtb2 top-to-bottom review: i left off at device. I was working through the enums the other day instead of the openrtb2 objects. I switched to match. i have a couple of comments/questions on how to handle some spec differences.

@SyntaxNode
Copy link
Contributor Author

I completed the top-to-bottom review up until the UID object. fixed some errata. should all be good now.

openrtb2/imp.go Outdated Show resolved Hide resolved
mxmCherry added 4 commits May 19, 2022 15:02
Mostly for enum shift (206 and higher IDs) introduced in:
InteractiveAdvertisingBureau/openrtb@5a6aac5

> 3.0 September 2021 Clarified loss reason: Loss Reason Code 206 was renamed from Bundle to Store ID to keep the intent of this loss reason consistent across OpenRTB 2.x and 3.x.
@mxmCherry mxmCherry self-requested a review May 19, 2022 12:55
Copy link
Owner

@mxmCherry mxmCherry left a comment

Choose a reason for hiding this comment

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

I consider this as good for now.

I'll merge it now, then will release a v16-alpha.

Maybe something will pop up when bumping this lib where used (maybe https://github.com/prebid/prebid-server etc).

@mxmCherry
Copy link
Owner

Note: addresses prebid/prebid-server#2139

@mxmCherry mxmCherry merged commit 19fd17c into mxmCherry:master May 19, 2022
@SyntaxNode
Copy link
Contributor Author

SyntaxNode commented May 19, 2022

I consider this as good for now.
I'll merge it now, then will release a v16-alpha.

Fantastic. Thank you for working with me on this PR.

Maybe something will pop up when bumping this lib where used (maybe https://github.com/prebid/prebid-server etc).

That's my project. :) I don't foresee any issues, but there may be surprises when we start going through the code.

@SyntaxNode SyntaxNode deleted the openrtb26 branch June 21, 2022 20:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants