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

Upgrade linters to ESLint with stricter code style #1111

Merged
merged 15 commits into from
May 17, 2017
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
{
"env": {
"browser": true,
"commonjs": true
},
"extends": "standard",
"globals": {
"$$PREBID_GLOBAL$$": false
},
"parserOptions": {
"sourceType": "module"
},
"rules": {
"comma-dangle": "off",
"semi": "off",
"space-before-function-paren": "off",

// Exceptions below this line are temporary, so that eslint can be added into the CI process.
// Violations of these styles should be fixed, and the exceptions removed over time.
//
// See Issue #1111.
"brace-style": "off",
"camelcase": "off",
"eqeqeq": "off",
"import/first": "off",
"no-control-regex": "off",
"no-mixed-operators": "off",
"no-multiple-empty-lines": "off",
"no-redeclare": "off",
"no-return-assign": "off",
"no-throw-literal": "off",
"no-undef": "off",
"no-unused-vars": "off",
"no-use-before-define": "off",
"no-useless-call": "off",
"no-useless-escape": "off",
"one-var": "off",
"standard/no-callback-literal": "off",
"standard/object-curly-even-spacing": "off",
"valid-typeof": "off"
}
}
11 changes: 0 additions & 11 deletions .jscsrc

This file was deleted.

39 changes: 0 additions & 39 deletions .jshintrc

This file was deleted.

4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ Please **do NOT load Prebid.js inside your adapter**. If you do this, we will re

### Code Quality

Code quality is defined by `.jscs` and `.jshint` files and errors are reported in the terminal.
Code quality is defined by `.eslintrc` and errors are reported in the terminal.

If you are contributing code, you should configure your editor with the provided `.jscs` and `.jshint` settings.
If you are contributing code, you should [configure your editor](http://eslint.org/docs/user-guide/integrations#editors) with the provided `.eslintrc` settings.

### Unit Testing with Karma

Expand Down
56 changes: 26 additions & 30 deletions gulpfile.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
'use strict';

var argv = require('yargs').argv;
var gulp = require('gulp');
var argv = require('yargs').argv;
var gutil = require('gulp-util');
var connect = require('gulp-connect');
var webpack = require('webpack-stream');
var uglify = require('gulp-uglify');
var jshint = require('gulp-jshint');
var clean = require('gulp-clean');
var karma = require('gulp-karma');
var mocha = require('gulp-mocha');
Expand All @@ -16,12 +13,12 @@ var helpers = require('./gulpHelpers');
var del = require('del');
var gulpJsdoc2md = require('gulp-jsdoc-to-markdown');
var concat = require('gulp-concat');
var jscs = require('gulp-jscs');
var header = require('gulp-header');
var zip = require('gulp-zip');
var replace = require('gulp-replace');
var shell = require('gulp-shell');
var optimizejs = require('gulp-optimize-js');
const eslint = require('gulp-eslint');

var CI_MODE = process.env.NODE_ENV === 'ci';
var prebid = require('./package.json');
Expand All @@ -32,13 +29,13 @@ var analyticsDirectory = '../analytics';
var port = 9999;

// Tasks
gulp.task('default', ['clean', 'quality', 'webpack']);
gulp.task('default', ['clean', 'lint', 'webpack']);

gulp.task('serve', ['clean', 'quality', 'devpack', 'webpack', 'watch', 'test']);
gulp.task('serve', ['clean', 'lint', 'devpack', 'webpack', 'watch', 'test']);

gulp.task('serve-nw', ['clean', 'quality', 'devpack', 'webpack', 'watch', 'e2etest']);
gulp.task('serve-nw', ['clean', 'lint', 'devpack', 'webpack', 'watch', 'e2etest']);

gulp.task('run-tests', ['clean', 'quality', 'webpack', 'test', 'mocha']);
gulp.task('run-tests', ['clean', 'lint', 'webpack', 'test', 'mocha']);

gulp.task('build', ['webpack']);

Expand All @@ -59,7 +56,7 @@ gulp.task('devpack', function () {
.pipe(connect.reload());
});

gulp.task('webpack', function () {
gulp.task('webpack', ['clean'], function () {

// change output filename if argument --tag given
if (argv.tag && argv.tag.length) {
Expand All @@ -80,7 +77,7 @@ gulp.task('webpack', function () {
});

//zip up for release
gulp.task('zip', ['jscs', 'clean', 'webpack'], function () {
gulp.task('zip', ['clean', 'webpack'], function () {
return gulp.src(['build/dist/*', 'integrationExamples/gpt/*'])
.pipe(zip(packageNameVersion + '.zip'))
.pipe(gulp.dest('./'));
Expand All @@ -89,7 +86,7 @@ gulp.task('zip', ['jscs', 'clean', 'webpack'], function () {
// Karma Continuous Testing
// Pass your browsers by using --browsers=chrome,firefox,ie9
// Run CI by passing --watch
gulp.task('test', function () {
gulp.task('test', ['clean'], function () {
var defaultBrowsers = CI_MODE ? ['PhantomJS'] : ['Chrome'];
var browserArgs = helpers.parseBrowserArgs(argv).map(helpers.toCapitalCase);

Expand Down Expand Up @@ -137,7 +134,16 @@ gulp.task('test', function () {
}));
});

gulp.task('mocha', ['webpack'], function() {
//
// Making this task depend on lint is a bit of a hack. The `run-tests` command is the entrypoint for the CI process,
// and it needs to run all these tasks together. However, the "lint" and "mocha" tasks explode when used in parallel,
// resulting in some mysterious "ShellJS: internal error TypeError: Cannot read property 'isFile' of undefined"
// errors.
//
// Gulp doesn't support serial dependencies (until gulp 4.0... which is most likely never coming out)... so we have
// to trick it by declaring 'lint' as a dependency here. See https://github.com/gulpjs/gulp/blob/master/docs/recipes/running-tasks-in-series.md
//
gulp.task('mocha', ['webpack', 'lint'], function() {
return gulp.src(['test/spec/loaders/**/*.js'], { read: false })
.pipe(mocha({
reporter: 'spec',
Expand Down Expand Up @@ -175,11 +181,11 @@ gulp.task('watch', function () {
'src/**/*.js',
'test/spec/**/*.js',
'!test/spec/loaders/**/*.js'
], ['quality', 'webpack', 'devpack', 'test']);
], ['lint', 'webpack', 'devpack', 'test']);
gulp.watch([
'loaders/**/*.js',
'test/spec/loaders/**/*.js'
], ['quality', 'mocha']);
], ['lint', 'mocha']);
gulp.watch(['integrationExamples/gpt/*.html'], ['test']);
connect.server({
https: argv.https,
Expand All @@ -189,21 +195,11 @@ gulp.task('watch', function () {
});
});

gulp.task('quality', ['hint', 'jscs']);

gulp.task('hint', function () {
return gulp.src('src/**/*.js')
.pipe(jshint('.jshintrc'))
.pipe(jshint.reporter('jshint-stylish'))
.pipe(jshint.reporter('fail'));
});

gulp.task('jscs', function () {
return gulp.src('src/**/*.js')
.pipe(jscs({
configPath: '.jscsrc'
}))
.pipe(jscs.reporter());
gulp.task('lint', () => {
return gulp.src(['src/**/*.js', 'test/**/*.js'])
.pipe(eslint())
.pipe(eslint.format('stylish'))
.pipe(eslint.failAfterError());
});

gulp.task('clean-docs', function () {
Expand Down
11 changes: 7 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,21 @@
"del": "^2.2.0",
"ejs": "^2.5.1",
"es5-shim": "^4.5.2",
"eslint-config-standard": "^10.2.0",
"eslint-plugin-import": "^2.2.0",
"eslint-plugin-node": "^4.2.2",
"eslint-plugin-promise": "^3.5.0",
"eslint-plugin-standard": "^3.0.1",
"faker": "^3.1.0",
"fs.extra": "^1.3.2",
"gulp": "^3.8.7",
"gulp-babel": "^6.1.2",
"gulp-clean": "^0.3.1",
"gulp-clean": "^0.3.2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we're updating dependencies, the yarn.lock file should probably be updated as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

good catch ^^. Done.

"gulp-concat": "^2.6.0",
"gulp-connect": "^2.0.6",
"gulp-eslint": "^3.0.1",
"gulp-header": "^1.7.1",
"gulp-jscs": "^3.0.2",
"gulp-jsdoc-to-markdown": "^1.2.1",
"gulp-jshint": "^1.8.4",
"gulp-karma": "0.0.4",
"gulp-mocha": "^2.2.0",
"gulp-optimize-js": "^1.1.0",
Expand All @@ -54,7 +58,6 @@
"gulp-zip": "^3.1.0",
"istanbul": "^0.3.2",
"istanbul-instrumenter-loader": "^0.1.2",
"jshint-stylish": "^2.2.1",
"json-loader": "^0.5.1",
"karma": "^0.13.2",
"karma-babel-preprocessor": "^6.0.1",
Expand Down
30 changes: 13 additions & 17 deletions src/adaptermanager.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { BaseAdapter } from './adapters/baseAdapter';
var _bidderRegistry = {};
exports.bidderRegistry = _bidderRegistry;

//create s2s settings objectType_function
// create s2s settings objectType_function
let _s2sConfig = {};
var _analyticsRegistry = {};
let _bidderSequence = null;
Expand All @@ -31,7 +31,7 @@ function getBids({bidderCode, requestId, bidderRequestId, adUnits}) {
return Object.assign({}, bid, {
placementCode: adUnit.code,
mediaType: adUnit.mediaType,
transactionId : adUnit.transactionId,
transactionId: adUnit.transactionId,
sizes: sizes,
bidId: bid.bid_id || utils.getUniqueIdentifierStr(),
bidderRequestId,
Expand All @@ -57,17 +57,17 @@ exports.callBids = ({adUnits, cbTimeout}) => {
bidderCodes = shuffle(bidderCodes);
}

if(_s2sConfig.enabled) {
//these are called on the s2s adapter
if (_s2sConfig.enabled) {
// these are called on the s2s adapter
let adaptersServerSide = _s2sConfig.bidders;

//don't call these client side
// don't call these client side
bidderCodes = bidderCodes.filter((elm) => {
return !adaptersServerSide.includes(elm);
});
let adUnitsCopy = utils.cloneJson(adUnits);

//filter out client side bids
// filter out client side bids
adUnitsCopy.forEach((adUnit) => {
if (adUnit.sizeMapping) {
adUnit.sizes = mapSizes(adUnit);
Expand All @@ -90,17 +90,17 @@ exports.callBids = ({adUnits, cbTimeout}) => {
requestId,
bidderRequestId,
tid,
bids: getBids({bidderCode, requestId, bidderRequestId, 'adUnits' : adUnitsCopy}),
bids: getBids({bidderCode, requestId, bidderRequestId, 'adUnits': adUnitsCopy}),
start: new Date().getTime(),
auctionStart: auctionStart,
timeout: _s2sConfig.timeout
};
//Pushing server side bidder
// Pushing server side bidder
$$PREBID_GLOBAL$$._bidsRequested.push(bidderRequest);
});

let s2sBidRequest = {tid, 'ad_units' : adUnitsCopy};
let s2sAdapter = _bidderRegistry[_s2sConfig.adapter]; //jshint ignore:line
let s2sBidRequest = {tid, 'ad_units': adUnitsCopy};
let s2sAdapter = _bidderRegistry[_s2sConfig.adapter]; // jshint ignore:line
utils.logMessage(`CALLING S2S HEADER BIDDERS ==== ${adaptersServerSide.join(',')}`);
s2sAdapter.setConfig(_s2sConfig);
s2sAdapter.callBids(s2sBidRequest);
Expand Down Expand Up @@ -138,8 +138,8 @@ function transformHeightWidth(adUnit) {
sizes.forEach(size => {
let heightWidth = size.split('x');
let sizeObj = {
'w' : parseInt(heightWidth[0]),
'h' : parseInt(heightWidth[1])
'w': parseInt(heightWidth[0]),
'h': parseInt(heightWidth[1])
};
sizesObj.push(sizeObj);
});
Expand All @@ -148,14 +148,11 @@ function transformHeightWidth(adUnit) {

exports.registerBidAdapter = function (bidAdaptor, bidderCode) {
if (bidAdaptor && bidderCode) {

if (typeof bidAdaptor.callBids === CONSTANTS.objectType_function) {
_bidderRegistry[bidderCode] = bidAdaptor;

} else {
utils.logError('Bidder adaptor error for bidder code: ' + bidderCode + 'bidder must implement a callBids() function');
}

} else {
utils.logError('bidAdaptor or bidderCode not specified');
}
Expand All @@ -173,7 +170,7 @@ exports.aliasBidAdapter = function (bidderCode, alias) {
try {
let newAdapter = null;
if (bidAdaptor instanceof BaseAdapter) {
//newAdapter = new bidAdaptor.constructor(alias);
// newAdapter = new bidAdaptor.constructor(alias);
utils.logError(bidderCode + ' bidder does not currently support aliasing.', 'adaptermanager.aliasBidAdapter');
} else {
newAdapter = bidAdaptor.createNew();
Expand All @@ -191,7 +188,6 @@ exports.aliasBidAdapter = function (bidderCode, alias) {

exports.registerAnalyticsAdapter = function ({adapter, code}) {
if (adapter && code) {

if (typeof adapter.enableAnalytics === CONSTANTS.objectType_function) {
adapter.code = code;
_analyticsRegistry[code] = adapter;
Expand Down
Loading