Skip to content

Commit

Permalink
Merge pull request #13 from prebid/master
Browse files Browse the repository at this point in the history
Sync with master
  • Loading branch information
pm-nitin-shirsat authored Jun 27, 2024
2 parents 14db919 + 2c8811a commit 8761239
Show file tree
Hide file tree
Showing 616 changed files with 37,327 additions and 43,038 deletions.
4 changes: 1 addition & 3 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ aliases:
- &environment
docker:
# specify the version you desire here
- image: cimg/node:16.20-browsers
- image: cimg/node:20.14.0-browsers
resource_class: xlarge
# Specify service dependencies here if necessary
# CircleCI maintains a library of pre-built images
Expand All @@ -18,8 +18,6 @@ aliases:
- &restore_dep_cache
keys:
- v1-dependencies-{{ checksum "package.json" }}
# fallback to using the latest cache if no exact match is found
- v1-dependencies-

- &save_dep_cache
paths:
Expand Down
2 changes: 1 addition & 1 deletion .devcontainer/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
ARG VARIANT="12"
ARG VARIANT="20"
FROM mcr.microsoft.com/vscode/devcontainers/javascript-node:${VARIANT}

RUN curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor > /usr/share/keyrings/yarn-archive-keyring.gpg
Expand Down
12 changes: 12 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ module.exports = {
files: key + '/**/*.js',
rules: {
'prebid/validate-imports': ['error', allowedModules[key]],
'prebid/no-innerText': ['error', allowedModules[key]],
'no-restricted-globals': [
'error',
{
Expand All @@ -95,5 +96,16 @@ module.exports = {
// code in other packages (such as plugins/eslint) is not "seen" by babel and its parser will complain.
files: 'plugins/*/**/*.js',
parser: 'esprima'
},
{
files: '**BidAdapter.js',
rules: {
'no-restricted-imports': [
'error', {
patterns: ["**/src/events.js",
"**/src/adloader.js"]
}
]
}
}])
};
124 changes: 124 additions & 0 deletions .github/workflows/jscpd.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
name: Check for Duplicated Code

on:
pull_request_target:
branches:
- master

jobs:
check-duplication:
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v4
with:
fetch-depth: 0 # Fetch all history for all branches
ref: ${{ github.event.pull_request.head.sha }}

- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: '20'

- name: Install dependencies
run: |
npm install -g jscpd diff-so-fancy
- name: Create jscpd config file
run: |
echo '{
"threshold": 20,
"minTokens": 50,
"reporters": [
"json"
],
"output": "./",
"pattern": "**/*.js",
"ignore": "**/*spec.js"
}' > .jscpd.json
- name: Run jscpd on entire codebase
run: jscpd

- name: Fetch base and target branches
run: |
git fetch origin +refs/heads/${{ github.event.pull_request.base.ref }}:refs/remotes/origin/${{ github.event.pull_request.base.ref }}
git fetch origin +refs/pull/${{ github.event.pull_request.number }}/merge:refs/remotes/pull/${{ github.event.pull_request.number }}/merge
- name: Get the diff
run: git diff --name-only origin/${{ github.event.pull_request.base.ref }}...refs/remotes/pull/${{ github.event.pull_request.number }}/merge > changed_files.txt

- name: List generated files (debug)
run: ls -l

- name: Upload unfiltered jscpd report
if: always()
uses: actions/upload-artifact@v4
with:
name: unfiltered-jscpd-report
path: ./jscpd-report.json

- name: Filter jscpd report for changed files
run: |
if [ ! -f ./jscpd-report.json ]; then
echo "jscpd-report.json not found"
exit 1
fi
echo "Filtering jscpd report for changed files..."
CHANGED_FILES=$(jq -R -s -c 'split("\n")[:-1]' changed_files.txt)
echo "Changed files: $CHANGED_FILES"
jq --argjson changed_files "$CHANGED_FILES" '
.duplicates | map(select(
(.firstFile?.name as $fname | $changed_files | any(. == $fname)) or
(.secondFile?.name as $sname | $changed_files | any(. == $sname))
))
' ./jscpd-report.json > filtered-jscpd-report.json
cat filtered-jscpd-report.json
- name: Check if filtered jscpd report exists
id: check_filtered_report
run: |
if [ $(wc -l < ./filtered-jscpd-report.json) -gt 1 ]; then
echo "filtered_report_exists=true" >> $GITHUB_ENV
else
echo "filtered_report_exists=false" >> $GITHUB_ENV
fi
- name: Upload filtered jscpd report
if: env.filtered_report_exists == 'true'
uses: actions/upload-artifact@v4
with:
name: filtered-jscpd-report
path: ./filtered-jscpd-report.json

- name: Post GitHub comment
if: env.filtered_report_exists == 'true'
uses: actions/github-script@v7
with:
script: |
const fs = require('fs');
const filteredReport = JSON.parse(fs.readFileSync('filtered-jscpd-report.json', 'utf8'));
let comment = "Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:\n\n";
function link(dup) {
return `https://github.com/${{ github.event.repository.full_name }}/blob/${{ github.event.pull_request.head.sha }}/${dup.name}#L${dup.start}-L${dup.end - 1}`
}
filteredReport.forEach(duplication => {
const firstFile = duplication.firstFile;
const secondFile = duplication.secondFile;
const lines = duplication.lines;
comment += `- [\`${firstFile.name}\`](${link(firstFile)}) has ${lines} duplicated lines with [\`${secondFile.name}\`](${link(secondFile)})\n`;
});
comment += "\nReducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀";
github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.issue.number,
body: comment
});
- name: Fail if duplications are found
if: env.filtered_report_exists == 'true'
run: |
echo "Duplications found, failing the check."
exit 1
2 changes: 1 addition & 1 deletion .nvmrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
12.16.1
20.13.1
17 changes: 10 additions & 7 deletions PR_REVIEW.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ General gulp commands include separate commands for serving the codebase on a bu
- Checkout the branch (these instructions are available on the GitHub PR page as well).
- Verify PR is a single change type. Example, refactor OR bugfix. If more than 1 type, ask submitter to break out requests.
- 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.
- Verify tests are green in Travis-ci + local build by running `gulp serve` | `gulp test`
- Verify tests are green in circle-ci + local build by running `gulp serve` | `gulp test`
- Verify no code quality violations are present from linting (should be reported in terminal)
- Make sure the code is not setting cookies or localstorage directly -- it must use the `StorageManager`.
- Review for obvious errors or bad coding practice / use best judgement here.
- 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.
- 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.
- 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.
- 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.
Expand All @@ -51,20 +52,21 @@ Follow steps above for general review process. In addition, please verify the fo
- If the adapter being submitted is an alias type, check with the bidder contact that is being aliased to make sure it's allowed.
- All bidder parameter conventions must be followed:
- Video params must be read from AdUnit.mediaTypes.video when available; however bidder config can override the ad unit.
- First party data must be read from [getConfig('ortb2');](https://docs.prebid.org/dev-docs/publisher-api-reference/setConfig.html#setConfig-fpd).
- First party data must be read from the bid request object: bidrequest.ortb2
- 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.
- Adapters cannot accept an schain parameter. Rather, they must look for the schain parameter at bidRequest.schain.
- The bidderRequest.refererInfo.referer must be checked in addition to any bidder-specific parameter.
- Page position must come from bidrequest.mediaTypes.banner.pos or bidrequest.mediaTypes.video.pos
- Global OpenRTB fields should come from [getConfig('ortb2');](https://docs.prebid.org/dev-docs/publisher-api-reference/setConfig.html#setConfig-fpd):
- Eids object is to be preferred to Userids object in the bid request, as the userid object may be removed in a future version
- Global OpenRTB fields should come from bidrequest.ortb2
- bcat, battr, badv
- Impression-specific OpenRTB fields should come from bidrequest.ortb2imp
- instl
- Below are some examples of bidder specific updates that should require docs update (in their dev-docs/bidders/BIDDER.md file):
- If they support the GDPR consentManagement module and TCF1, add `gdpr_supported: true`
- If they support the GDPR consentManagement module and TCF2, add `tcf2_supported: true`
- If they support the TCF consentManagementTcf module and TCF2, add `tcf2_supported: true`
- If they support the US Privacy consentManagementUsp module, add `usp_supported: true`
- If they support one or more userId modules, add `userId: (list of supported vendors)`
- If they support the GPP consentManagementGpp module, add `gpp_supported: true`
- If they support one or more userId modules, add `userId: (list of supported vendors) or (all)`
- 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`
- If they support COPPA, add `coppa_supported: true`
- If they support SChain, add `schain_supported: true`
Expand Down Expand Up @@ -100,7 +102,7 @@ Follow steps above for general review process. In addition:
- modules/userId/userId.md
- tests can go either within the userId_spec.js file or in their own _spec file if they wish
- GVLID is recommended in the *IdSystem file if they operate in EU
- 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)
- 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
- the 3 available methods (getId, extendId, decode) should be used as they were intended
- decode (required method) should not be making requests to retrieve a new ID, it should just be decoding a response
- extendId (optional method) should not be making requests to retrieve a new ID, it should just be adding additional data to the id object
Expand All @@ -121,6 +123,7 @@ Follow steps above for general review process. In addition:
- Confirm that the module
- is not loading external code. If it is, escalate to the #prebid-js Slack channel.
- is reading `config` from the function signature rather than calling `getConfig`.
- Is practicing reasonable data minimization, eg not sending all eids over the wire without publisher whitelisting
- is sending data to the bid request only as either First Party Data or in bidRequest.rtd.RTDPROVIDERCODE.
- is making HTTPS requests as early as possible, but not more often than needed.
- doesn't force bid adapters to load additional code.
Expand Down
1 change: 0 additions & 1 deletion allowedModules.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@

module.exports = {
'modules': [
'criteo-direct-rsa-validate',
'crypto-js',
'live-connect' // Maintained by LiveIntent : https://github.com/liveintent-berlin/live-connect/
],
Expand Down
26 changes: 13 additions & 13 deletions browsers.json
Original file line number Diff line number Diff line change
@@ -1,51 +1,51 @@
{
"bs_edge_latest_windows_10": {
"bs_edge_latest_windows_11": {
"base": "BrowserStack",
"os_version": "10",
"os_version": "11",
"browser": "edge",
"browser_version": "latest",
"device": null,
"os": "Windows"
},
"bs_chrome_latest_windows_10": {
"bs_chrome_latest_windows_11": {
"base": "BrowserStack",
"os_version": "10",
"os_version": "11",
"browser": "chrome",
"browser_version": "latest",
"device": null,
"os": "Windows"
},
"bs_chrome_87_windows_10": {
"bs_chrome_107_windows_10": {
"base": "BrowserStack",
"os_version": "10",
"browser": "chrome",
"browser_version": "87.0",
"browser_version": "107.0",
"device": null,
"os": "Windows"
},
"bs_firefox_latest_windows_10": {
"bs_firefox_latest_windows_11": {
"base": "BrowserStack",
"os_version": "10",
"os_version": "11",
"browser": "firefox",
"browser_version": "latest",
"device": null,
"os": "Windows"
},
"bs_safari_latest_mac_bigsur": {
"bs_safari_latest_mac": {
"base": "BrowserStack",
"os_version": "Big Sur",
"os_version": "Sonoma",
"browser": "safari",
"browser_version": "latest",
"device": null,
"os": "OS X"
},
"bs_safari_15_catalina": {
"base": "BrowserStack",
"os_version": "Catalina",
"os_version": "Monterey",
"browser": "safari",
"browser_version": "13.1",
"browser_version": "15.6",
"device": null,
"os": "OS X"
}

}
43 changes: 25 additions & 18 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,17 @@ function makeVerbose(config = webpackConfig) {
});
}

function prebidSource(webpackCfg) {
var externalModules = helpers.getArgModules();

const analyticsSources = helpers.getAnalyticsSources();
const moduleSources = helpers.getModulePaths(externalModules);

return gulp.src([].concat(moduleSources, analyticsSources, 'src/prebid.js'))
.pipe(helpers.nameModules(externalModules))
.pipe(webpackStream(webpackCfg, webpack));
}

function makeDevpackPkg(config = webpackConfig) {
return function() {
var cloned = _.cloneDeep(config);
Expand All @@ -163,14 +174,7 @@ function makeDevpackPkg(config = webpackConfig) {
.filter((use) => use.loader === 'babel-loader')
.forEach((use) => use.options = Object.assign({}, use.options, babelConfig));

var externalModules = helpers.getArgModules();

const analyticsSources = helpers.getAnalyticsSources();
const moduleSources = helpers.getModulePaths(externalModules);

return gulp.src([].concat(moduleSources, analyticsSources, 'src/prebid.js'))
.pipe(helpers.nameModules(externalModules))
.pipe(webpackStream(cloned, webpack))
return prebidSource(cloned)
.pipe(gulp.dest('build/dev'))
.pipe(connect.reload());
}
Expand All @@ -183,14 +187,7 @@ function makeWebpackPkg(config = webpackConfig) {
}

return function buildBundle() {
var externalModules = helpers.getArgModules();

const analyticsSources = helpers.getAnalyticsSources();
const moduleSources = helpers.getModulePaths(externalModules);

return gulp.src([].concat(moduleSources, analyticsSources, 'src/prebid.js'))
.pipe(helpers.nameModules(externalModules))
.pipe(webpackStream(cloned, webpack))
return prebidSource(cloned)
.pipe(gulp.dest('build/dist'));
}
}
Expand Down Expand Up @@ -413,7 +410,9 @@ function runKarma(options, done) {
// the karma server appears to leak memory; starting it multiple times in a row will run out of heap
// here we run it in a separate process to bypass the problem
options = Object.assign({browsers: helpers.parseBrowserArgs(argv)}, options)
const child = fork('./karmaRunner.js');
const child = fork('./karmaRunner.js', null, {
env: Object.assign({}, options.env, process.env)
});
child.on('exit', (exitCode) => {
if (exitCode) {
done(new Error('Karma tests failed with exit code ' + exitCode));
Expand All @@ -426,7 +425,15 @@ function runKarma(options, done) {

// If --file "<path-to-test-file>" is given, the task will only run tests in the specified file.
function testCoverage(done) {
runKarma({coverage: true, browserstack: false, watch: false, file: argv.file}, done);
runKarma({
coverage: true,
browserstack: false,
watch: false,
file: argv.file,
env: {
NODE_OPTIONS: '--max-old-space-size=8096'
}
}, done);
}

function coveralls() { // 2nd arg is a dependency: 'test' must be finished
Expand Down
Loading

0 comments on commit 8761239

Please sign in to comment.