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

FPD enrichment module docs #2910

Merged
merged 6 commits into from
May 25, 2021
Merged

FPD enrichment module docs #2910

merged 6 commits into from
May 25, 2021

Conversation

bretg
Copy link
Contributor

@bretg bretg commented Apr 27, 2021

Docs PR for prebid/Prebid.js#6452

@bretg bretg requested a review from patmmccann April 27, 2021 20:27
| viewport width | device.w | Hunts for window.innerWidth, window.document.documentElement.clientWidth, window.document.body.clientWidth |
| viewport height | device.w | Hunts for window.innerHeight, window.document.documentElement.clientHeight, window.document.body.clientHeight |
| meta keywords | site.keywords | Looks for a meta tag with name=keywords |

Copy link
Collaborator

Choose a reason for hiding this comment

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

do we imagine future enrichments would happen in new submodules or in the current submodule behind config? For example parsing timezone into country or copying the floc from eid to device object or video ad unit enrichment

Could affect how this is worded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My take is case-by-case. If the enrichment is cheap, we can stick it in here. If it's expensive (code size and processing) it should live in a new module.

What wording would it affect?

Copy link
Collaborator

@patmmccann patmmccann May 6, 2021

Choose a reason for hiding this comment

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

Largely just that this is "the" enrichment submodule as opposed to language suggesting first of several

@bretg
Copy link
Contributor Author

bretg commented May 5, 2021

I need to update this to reflect the split into sub-modules

@bretg bretg requested a review from patmmccann May 11, 2021 20:45
@patmmccann
Copy link
Collaborator

This seems to suggest the split is into two different modules instead of two submodules of one module

@patmmccann
Copy link
Collaborator

discussed in taxonomy committee, this looks good as is and we'll handle docs / name changes as new submodules come

@bretg bretg merged commit 420d7bc into master May 25, 2021
@bretg bretg deleted the fpd-module branch May 25, 2021 19:32
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.

2 participants