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

adding PR_REVIEW guideline #2159

Merged
merged 3 commits into from
Mar 6, 2018
Merged

adding PR_REVIEW guideline #2159

merged 3 commits into from
Mar 6, 2018

Conversation

bretg
Copy link
Collaborator

@bretg bretg commented Feb 16, 2018

Adapters may not globally override or default the standard ad server targeting values: hb_adid, hb_bidder, hb_pb, hb_deal, hb_size

Type of change

  • Bugfix
  • Feature
  • New bidder adapter
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Does this change affect user-facing APIs or examples documented on http://prebid.org?
  • Other

Description of change

Added a guideline to the PR Review guidelines

Adapters may not globally override or default the standard ad server targeting values: hb_adid, hb_bidder, hb_pb, hb_deal, hb_size
@bretg
Copy link
Collaborator Author

bretg commented Feb 16, 2018

Docs PR - prebid/prebid.github.io#621

PR_REVIEW.md Outdated
@@ -33,6 +33,7 @@ For modules and core platform updates, the initial reviewer should request an ad
- All user-sync (aka pixel) activity must be registered via the provided functions
- Adapters may not use the $$PREBID_GLOBAL$$ variable
- All adapters must support the creation of multiple concurrent instances. This means, for example, that adapters cannot rely on mutable global variables.
- Adapters may not globally override or default the standard ad server targeting values: hb_adid, hb_bidder, hb_pb, hb_deal, hb_size.
Copy link
Member

@mkendall07 mkendall07 Feb 16, 2018

Choose a reason for hiding this comment

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

I think we added hb_format too (as a standard key)

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.

needs the additional keys added or a link to source

@mkendall07 mkendall07 added this to the Prebid 1.5.0 milestone Feb 23, 2018
@matthewlane
Copy link
Collaborator

@mkendall07 Updated to match docs PR, please recheck

@matthewlane matthewlane merged commit fa55342 into master Mar 6, 2018
@matthewlane matthewlane deleted the new-review-guideline branch March 6, 2018 17:53
mizmaar3 added a commit to widespace-os/Prebid.js that referenced this pull request Mar 8, 2018
* master: (27 commits)
  Increment pre version
  Prebid 1.5.0 Release
  Fix cross-platform test failures (prebid#2228)
  Fix uncahced video bids from multi-response array triggering callback early (prebid#2219)
  Add vuble adapter (prebid#2201)
  Update Vidazoo domain (prebid#2223)
  InSkin Bid Adapter: remove referrer field from request body (prebid#2217)
  Gamma Support UserSync Endpoint (prebid#2216)
  Feature/stale bot (prebid#2192)
  33Across Bid Adapter: updated user sync endpoint (prebid#2193)
  Adding PR_REVIEW guideline (prebid#2159)
  Add FairTrade Bid Adapter (prebid#2147)
  Add banner support to Beachfront adapter (prebid#2117)
  Smartyads Adapter 1.x (prebid#2080)
  Audience Network: allow native bids for non-IAB sizes (prebid#2203)
  Update position default value in rubicon (prebid#2214)
  Auctionmanager spec refactor pr (prebid#2155)
  fix mediaType being overwritten by undefined in rubicon bid adapter (prebid#2209)
  Fix lint error (prebid#2208)
  VAST support in adform adapter (prebid#2173)
  ...
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
* adding PR_REVIEW guideline

Adapters may not globally override or default the standard ad server targeting values: hb_adid, hb_bidder, hb_pb, hb_deal, hb_size

* Update PR_REVIEW.md

* Update PR_REVIEW.md
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.

3 participants