You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy file name to clipboardExpand all lines: PR_REVIEW.md
+10-7
Original file line number
Diff line number
Diff line change
@@ -23,10 +23,11 @@ General gulp commands include separate commands for serving the codebase on a bu
23
23
- Checkout the branch (these instructions are available on the GitHub PR page as well).
24
24
- Verify PR is a single change type. Example, refactor OR bugfix. If more than 1 type, ask submitter to break out requests.
25
25
- Verify code under review has at least 80% unit test coverage. If legacy code doesn't have enough unit test coverage, require that additional unit tests to be included in the PR.
26
-
- Verify tests are green in Travis-ci + local build by running `gulp serve` | `gulp test`
26
+
- Verify tests are green in circle-ci + local build by running `gulp serve` | `gulp test`
27
27
- Verify no code quality violations are present from linting (should be reported in terminal)
28
28
- Make sure the code is not setting cookies or localstorage directly -- it must use the `StorageManager`.
29
29
- Review for obvious errors or bad coding practice / use best judgement here.
30
+
- Don't allow needless code duplication with other js files; require both files import common code. Do not allow commits designed to fool the code duplication checker.
30
31
- If the change is a new feature / change to core prebid.js - review the change with a Tech Lead on the project and make sure they agree with the nature of change.
31
32
- If the change results in needing updates to docs (such as public API change, module interface etc), add a label for "needs docs" and inform the submitter they must submit a docs PR to update the appropriate area of Prebid.org **before the PR can merge**. Help them with finding where the docs are located on prebid.org if needed.
32
33
- If all above is good, add a `LGTM` comment and, if the change is in PBS-core or is an important module like the prebidServerBidAdapter, request 1 additional core member to review.
@@ -51,20 +52,21 @@ Follow steps above for general review process. In addition, please verify the fo
51
52
- If the adapter being submitted is an alias type, check with the bidder contact that is being aliased to make sure it's allowed.
52
53
- All bidder parameter conventions must be followed:
53
54
- Video params must be read from AdUnit.mediaTypes.video when available; however bidder config can override the ad unit.
54
-
- First party data must be read from [getConfig('ortb2');](https://docs.prebid.org/dev-docs/publisher-api-reference/setConfig.html#setConfig-fpd).
55
+
- First party data must be read from the bid request object: bidrequest.ortb2
55
56
- Adapters that accept a floor parameter must also support the [floors module](https://docs.prebid.org/dev-docs/modules/floors.html) -- look for a call to the `getFloor()` function.
56
57
- Adapters cannot accept an schain parameter. Rather, they must look for the schain parameter at bidRequest.schain.
57
58
- The bidderRequest.refererInfo.referer must be checked in addition to any bidder-specific parameter.
58
59
- Page position must come from bidrequest.mediaTypes.banner.pos or bidrequest.mediaTypes.video.pos
59
-
- Global OpenRTB fields should come from [getConfig('ortb2');](https://docs.prebid.org/dev-docs/publisher-api-reference/setConfig.html#setConfig-fpd):
60
+
- Eids object is to be preferred to Userids object in the bid request, as the userid object may be removed in a future version
61
+
- Global OpenRTB fields should come from bidrequest.ortb2
60
62
- bcat, battr, badv
61
63
- Impression-specific OpenRTB fields should come from bidrequest.ortb2imp
62
64
- instl
63
65
- Below are some examples of bidder specific updates that should require docs update (in their dev-docs/bidders/BIDDER.md file):
64
-
- If they support the GDPR consentManagement module and TCF1, add `gdpr_supported: true`
65
-
- If they support the GDPR consentManagement module and TCF2, add `tcf2_supported: true`
66
+
- If they support the TCF consentManagementTcf module and TCF2, add `tcf2_supported: true`
66
67
- If they support the US Privacy consentManagementUsp module, add `usp_supported: true`
67
-
- If they support one or more userId modules, add `userId: (list of supported vendors)`
68
+
- If they support the GPP consentManagementGpp module, add `gpp_supported: true`
69
+
- If they support one or more userId modules, add `userId: (list of supported vendors) or (all)`
68
70
- If they support video and/or native mediaTypes add `media_types: video, native`. Note that display is added by default. If you don't support display, add "no-display" as the first entry, e.g. `media_types: no-display, native`
69
71
- If they support COPPA, add `coppa_supported: true`
70
72
- If they support SChain, add `schain_supported: true`
@@ -100,7 +102,7 @@ Follow steps above for general review process. In addition:
100
102
- modules/userId/userId.md
101
103
- tests can go either within the userId_spec.js file or in their own _spec file if they wish
102
104
- GVLID is recommended in the *IdSystem file if they operate in EU
103
-
- make sure example configurations align to the actual code (some modules use the userId storage settings and allow pub configuration, while others handle reading/writing cookies on their own, so should not include the storage params in examples)
105
+
- make sure example configurations align to the actual code (some modules use the userId storage settings and allow pub configuration, while others handle reading/writing cookies on their own, so should not include the storage params in examples). This ability to write will be removed in a future version, see https://github.com/prebid/Prebid.js/issues/10710
104
106
- the 3 available methods (getId, extendId, decode) should be used as they were intended
105
107
- decode (required method) should not be making requests to retrieve a new ID, it should just be decoding a response
106
108
- extendId (optional method) should not be making requests to retrieve a new ID, it should just be adding additional data to the id object
@@ -121,6 +123,7 @@ Follow steps above for general review process. In addition:
121
123
- Confirm that the module
122
124
- is not loading external code. If it is, escalate to the #prebid-js Slack channel.
123
125
- is reading `config` from the function signature rather than calling `getConfig`.
126
+
- Is practicing reasonable data minimization, eg not sending all eids over the wire without publisher whitelisting
124
127
- is sending data to the bid request only as either First Party Data or in bidRequest.rtd.RTDPROVIDERCODE.
125
128
- is making HTTPS requests as early as possible, but not more often than needed.
126
129
- doesn't force bid adapters to load additional code.
0 commit comments