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

create some hooks and modify adServerManager function #3438

Merged
merged 13 commits into from
Feb 6, 2019

Conversation

jsnellbaker
Copy link
Collaborator

Type of change

  • Feature
  • Refactoring (no functional changes, no api changes)

Description of change

This PR introduces a set of changes that's meant to lay-out some of the groundwork for #3379

Summary of the changes:

  • several functions are changed to hook functions
  • export several functions
  • renaming of the checkBidRequestSizes function to more generic name
  • removed hard-coded reference to buildVideoUrl property in adServerManager.js; now will support any property that's registered.
  • various unit test updates as a result of the above changes

@mkendall07 mkendall07 self-assigned this Jan 14, 2019
src/prebid.js Outdated
}

if (mediaTypes && mediaTypes.native) {
const native = mediaTypes.native;
Copy link
Collaborator

Choose a reason for hiding this comment

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

native as an identifier or object property can cause obscure issues because it was a reserved keyword in ECMAScript v3 -- may be a good time to update these references (in source files only, tests are fine). See #1621 for more info

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the heads-up; I pushed a commit to rename this variable.

Copy link
Member

@mkendall07 mkendall07 left a comment

Choose a reason for hiding this comment

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

LGTM

@jsnellbaker jsnellbaker merged commit 0ec9970 into master Feb 6, 2019
@jsnellbaker jsnellbaker deleted the longform_groundwork branch March 18, 2019 13:38
jacekburys-quantcast pushed a commit to jacekburys-quantcast/Prebid.js that referenced this pull request May 15, 2019
* create hooks and modify adservermanager function

* remove comment

* modify the import path for hook file

* move checkAdUnitSetup and refactor video.js hook

* update hook code to use new API

* change hooks to use sync type

* add adpod mediatype and rename native variable

* remove unused import from test file

* remove adpod reference in utils to avoid merge conflict
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants