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

OD-12994: DFP ad unit code #4

Merged
merged 1 commit into from
Jun 15, 2021

Conversation

Konstantseven
Copy link
Contributor

Added field for DFP ad unit code and GPID propagation

@@ -150,6 +150,9 @@ message ImpExt {
// Supported viewability vendors. For list of supported vendors, visit:
// https://resources.rubiconproject.com/resource/publisher-resources/xapi-specifications/#4.9.
repeated string viewabilityvendors = 3;

// Ad unit code.

Choose a reason for hiding this comment

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

Please update the comment to something like: Global Placement ID, as defined by TheTradeDesk. The goal is that all participants in buying impressions for the same placement get the same identifier, and that the identifier (as much as possible) is unique for the placement. (In general, this field will be populated with one of the DFP Ad Unit Code, Prebid Ad Slot, or some value provided by the publisher or ad server.)

Copy link
Collaborator

@eskenny eskenny left a comment

Choose a reason for hiding this comment

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

A few grammatical comments. I'll defer to others for the actual contents, but should we mention TTD?

@@ -150,6 +150,12 @@ message ImpExt {
// Supported viewability vendors. For list of supported vendors, visit:
// https://resources.rubiconproject.com/resource/publisher-resources/xapi-specifications/#4.9.
repeated string viewabilityvendors = 3;

// Global Placement ID. The unique identifier of the placement, that is sent
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be no comma after "placement"

the placement that is sent

@@ -150,6 +150,12 @@ message ImpExt {
// Supported viewability vendors. For list of supported vendors, visit:
// https://resources.rubiconproject.com/resource/publisher-resources/xapi-specifications/#4.9.
repeated string viewabilityvendors = 3;

// Global Placement ID. The unique identifier of the placement, that is sent
// to all participants in buying impressions for this placement (in general,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Parenthesis should be the start of a new sentence

for this placement. (In general,

// Global Placement ID. The unique identifier of the placement, that is sent
// to all participants in buying impressions for this placement (in general,
// this field will be populated with one of the DFP Ad Unit Code, Prebid Ad Slot,
// or value, provided by the publisher or ad server).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be no comma after value

// or value provided by

@eskenny eskenny merged commit 22f50d9 into MagniteEngineering:main Jun 15, 2021
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.

4 participants