Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 7 additions & 3 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
/optimize
/src/fixtures/vislib/mock_data
/src/ui/public/angular-bootstrap
/test/fixtures/scenarios
/src/core_plugins/console/public/webpackShims
/src/core_plugins/console/public/tests/webpackShims
/src/core_plugins/timelion/bower_components
/src/core_plugins/timelion/vendor_components
test/fixtures/scenarios
optimize
test/fixtures/scenarios
/src/ui/public/utils/decode_geo_hash.js
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this specific file excluded?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's copied from an external source

6 changes: 6 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
@@ -1,2 +1,8 @@
---
extends: '@elastic/kibana'
rules:
no-unused-vars: off
no-var: off
prefer-const: off
no-extra-semi: off
quotes: off
12 changes: 0 additions & 12 deletions Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,6 @@ module.exports = function (grunt) {
' * Copyright (c) <%= grunt.template.today("yyyy") %> <%= package.author.company %>;' +
' Licensed <%= package.license %> */\n'
},

lintThese: [
'Gruntfile.js',
'<%= root %>/tasks/**/*.js',
'<%= root %>/test/**/*.js',
'<%= src %>/**/*.js',
'!<%= src %>/ui/public/angular-bootstrap/**/*.js',
'!<%= src %>/core_plugins/timelion/bower_components/**/*.js',
'!<%= src %>/core_plugins/timelion/vendor_components/**/*.js',
'!<%= src %>/fixtures/**/*.js',
'!<%= root %>/test/fixtures/scenarios/**/*.js'
]
};

grunt.config.merge(config);
Expand Down
9 changes: 4 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -165,17 +165,17 @@
"wreck": "6.2.0"
},
"devDependencies": {
"@elastic/eslint-config-kibana": "0.0.3",
"@elastic/eslint-config-kibana": "0.2.1",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Once all of this eslint stuff is done, let's bump this to 1.0.0.

"angular-mocks": "1.4.7",
"auto-release-sinon": "1.0.3",
"babel-eslint": "4.1.8",
"babel-eslint": "6.1.2",
"chai": "3.5.0",
"cheerio": "0.22.0",
"chokidar": "1.6.0",
"chromedriver": "2.24.1",
"elasticdump": "2.1.1",
"eslint": "1.10.3",
"eslint-plugin-mocha": "1.1.0",
"eslint": "3.11.1",
"eslint-plugin-mocha": "4.7.0",
"event-stream": "3.3.2",
"expect.js": "0.3.1",
"faker": "1.1.0",
Expand All @@ -189,7 +189,6 @@
"grunt-karma": "2.0.0",
"grunt-run": "0.6.0",
"grunt-simple-mocha": "0.4.0",
"gruntify-eslint": "1.0.1",
"gulp-sourcemaps": "1.7.3",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎉

"handlebars": "4.0.5",
"husky": "0.8.1",
Expand Down
4 changes: 2 additions & 2 deletions src/cli/cluster/__tests__/_mock_cluster_fork.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export default class MockClusterFork extends EventEmitter {
dead = true;
this.emit('exit');
cluster.emit('exit', this, this.exitCode || 0);
}());
})();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How did this ever work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The babel parser missed the standard there apparently. it's valid for function expressions, just not arrow function expressions.

}),
},
isDead: sinon.spy(() => dead),
Expand All @@ -39,6 +39,6 @@ export default class MockClusterFork extends EventEmitter {
await wait();
dead = false;
this.emit('online');
}());
})();
}
}
23 changes: 10 additions & 13 deletions src/cli/cluster/cluster_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,19 +86,16 @@ module.exports = class ClusterManager {
const chokidar = require('chokidar');
const fromRoot = require('../../utils/from_root');

const watchPaths = uniq(
[
fromRoot('src/core_plugins'),
fromRoot('src/server'),
fromRoot('src/ui'),
fromRoot('src/utils'),
fromRoot('config'),
...extraPaths
]
.map(path => resolve(path))
);

this.watcher = chokidar.watch(watchPaths, {
const watchPaths = [
fromRoot('src/core_plugins'),
fromRoot('src/server'),
fromRoot('src/ui'),
fromRoot('src/utils'),
fromRoot('config'),
...extraPaths
].map(path => resolve(path));

this.watcher = chokidar.watch(uniq(watchPaths), {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Really nice attention to detail dude... I ❤️ it

cwd: fromRoot('.'),
ignored: /[\\\/](\..*|node_modules|bower_components|public|__tests__)[\\\/]/
});
Expand Down
60 changes: 31 additions & 29 deletions src/core_plugins/console/.eslintrc
Original file line number Diff line number Diff line change
@@ -1,34 +1,36 @@
---

root: true
extends: '@elastic/kibana'
extends: '../../../.eslintrc'

rules:
block-scoped-var: [0]
camelcase: [0]
curly: [0]
dot-location: [0]
dot-notation: [0]
eqeqeq: [0]
guard-for-in: [0]
indent: [0]
max-len: [0]
new-cap: [0]
no-caller: [0]
no-empty: [0]
no-extend-native: [0]
no-loop-func: [0]
no-multi-str: [0]
no-nested-ternary: [0]
no-proto: [0]
no-sequences: [0]
no-undef: [0]
no-use-before-define: [0]
one-var: [0]
quotes: [0]
space-before-blocks: [0]
space-in-parens: [0]
space-infix-ops: [0]
semi: [0]
strict: [0]
wrap-iife: [0]
block-scoped-var: off
camelcase: off
curly: off
dot-location: off
dot-notation: off
eqeqeq: off
guard-for-in: off
indent: off
max-len: off
new-cap: off
no-caller: off
no-empty: off
no-extend-native: off
no-loop-func: off
no-multi-str: off
no-nested-ternary: off
no-proto: off
no-sequences: off
no-undef: off
no-use-before-define: off
one-var: off
quotes: off
space-before-blocks: off
space-in-parens: off
space-infix-ops: off
semi: off
strict: off
wrap-iife: off
no-var: off
prefer-const: off
68 changes: 0 additions & 68 deletions src/core_plugins/timelion/.eslintrc

This file was deleted.

2 changes: 1 addition & 1 deletion src/fixtures/mock_ui_state.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ export default {
},
on: _.noop,
off: _.noop
}
};
10 changes: 5 additions & 5 deletions src/ui/ui_bundle_collection.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
const rimraf = promisify(require('rimraf'));
const mkdirp = promisify(require('mkdirp'));
const unlink = promisify(require('fs').unlink);
const readdir = promisify(require('fs').readdir);

import UiBundle from './ui_bundle';
import appEntryTemplate from './app_entry_template';
import { readFileSync as readSync } from 'fs';
import { pull, transform, pluck } from 'lodash';
import { promisify } from 'bluebird';
import { makeRe } from 'minimatch';

const rimraf = promisify(require('rimraf'));
const mkdirp = promisify(require('mkdirp'));
const unlink = promisify(require('fs').unlink);
const readdir = promisify(require('fs').readdir);

class UiBundleCollection {
constructor(bundlerEnv, filter) {
this.each = [];
Expand Down
57 changes: 30 additions & 27 deletions tasks/config/eslint.js
Original file line number Diff line number Diff line change
@@ -1,30 +1,33 @@
var resolve = require('path').resolve;
import { resolve } from 'path';
export default grunt => ({
options: {
paths: [
'Gruntfile.js',
'bin',
'config',
'src',
'tasks',
'test',
'utilities',
],
},

module.exports = function (grunt) {
return {
// just lint the source dir
source: {
options: {
cache: resolve(grunt.config.get('root'), '.eslint.fixSource.cache')
},
source: {
options: {
cache: resolve(grunt.config.get('root'), '.eslint.fixSource.cache')
}
},

files: {
src: '<%= lintThese %>'
}
},
fixSource: {
options: {
cache: resolve(grunt.config.get('root'), '.eslint.fixSource.cache'),
fix: true
}
},

// lint the source and fix any fixable errors
fixSource: {
options: {
cache: resolve(grunt.config.get('root'), '.eslint.fixSource.cache'),
fix: true
},

files: {
src: '<%= lintThese %>'
}
},

staged: {}
};
};
staged: {
options: {
paths: null // overridden by lintStagedFiles task
}
}
});
44 changes: 44 additions & 0 deletions tasks/eslint.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { CLIEngine } from 'eslint';

const OPTION_DEFAULTS = {
paths: null,
cache: null,
fix: false
};

export default grunt => {
grunt.registerMultiTask('eslint', function () {
const options = this.options(OPTION_DEFAULTS);

if (!options.paths) {
grunt.fatal(new Error('No eslint.options.paths specified'));
return;
}

const cli = new CLIEngine({
cache: options.cache,
fix: options.fix,
cwd: grunt.config.get('root'),
});

// report includes an entire list of files checked and the
// fixes, errors, and warning for each.
const report = cli.executeOnFiles(options.paths);

// output fixes to disk
if (options.fix) {
CLIEngine.outputFixes(report);
}

// log the formatted linting report
const formatter = cli.getFormatter();

const errTypes = [];
if (report.errorCount > 0) errTypes.push('errors');
if (report.warningCount > 0) errTypes.push('warning');
if (!errTypes.length) return;

grunt.log.write(formatter(report.results));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we get to this point if there are no errors? If there are no errors, do we still want to see the report results? Without knowing what the report contains, it seems like this could be useful information?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The report actually only contains errors and warnings, so if there are none of those then there isn't anything to report

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds good. How about renamingreport to errorReport or adding a comment to make that clearer? Otherwise I can see how someone might have the same question I had from reading this code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a comment describing what the report is. Since it contains errors, warnings, and fix info, errorReport didn't seem appropriate.

grunt.fatal(`eslint ${errTypes.join(' & ')}`);
});
};
Loading