Skip to content
This repository has been archived by the owner on Jun 18, 2021. It is now read-only.

eslint: add eslint to nodereport #46

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

eslint: add eslint to nodereport #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions .eslintrc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
env:
node: true
es6: true

rules:
indent: [2, 2]
quotes: [2, single]
no-trailing-spaces: 2
comma-spacing: [2, { before: false, after: true }]
eqeqeq: [2, always]
linebreak-style: [2, unix]
semi: [2, always]
no-console: 0
no-inner-declarations: 0
no-constant-condition: 0
no-regex-spaces: 0

extends: eslint:recommended
Copy link
Contributor

Choose a reason for hiding this comment

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

this isn't the same as nodejs/node: https://github.com/nodejs/node/blob/master/.eslintrc.yaml

Pretty much everyone from Node in a recent thread (that I can't find at the moment, sorry) heavily favours explicit specification, its quite painful updating between eslint versions when using their builtins.

Copy link
Member Author

@gdams gdams Jan 31, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Most of the other modules such as citgm don't currently hard code all the rules. The thread was a PR that I submitted the other day. I'm happy to define them all but is it really necessary for a module that isn't primarily javascript anyway? If we implement a CI such as travis we would very quickly pick up new rules that break things.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the easy thing to just directly copy node's file? I've had horrendous times in the past when eslint bumps majors trying to update, and they used to bump majors or introduce incompatibilities pretty often. From what I saw, I'm not the only one. If its easy, I would just copy the file from nodejs, if that is more complex than it seems (perhaps it relies on some custom rules?), then I've no objection to this. Any eslint is better than none.

Copy link
Member Author

Choose a reason for hiding this comment

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

Don't hold me to it, but I believe that the community uses several custom rules, i'll double check.

Copy link
Member

Choose a reason for hiding this comment

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

They do, it's in tools/eslint-rules.

I don't think it makes a big difference whether we include the rules or not, it shouldn't be that hard to include them if it means possibly less work down the line.

10 changes: 5 additions & 5 deletions demo/api_call.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
// Example - generation of NodeReport via API call
var nodereport = require('nodereport');
var http = require("http");
var http = require('http');

var count = 0;

function my_listener(request, response) {
switch(count++) {
case 0:
response.writeHead(200,{"Content-Type": "text/plain"});
response.write("\nRunning NodeReport API demo... refresh page to trigger NodeReport");
response.writeHead(200, {'Content-Type': 'text/plain'});
response.write('\nRunning NodeReport API demo... refresh page to trigger NodeReport');
response.end();
break;
case 1:
response.writeHead(200,{"Content-Type": "text/plain"});
response.writeHead(200, {'Content-Type': 'text/plain'});
// Call the nodereport module to trigger a NodeReport
var filename = nodereport.triggerReport();
response.write("\n" + filename + " written - refresh page to close");
response.write('\n' + filename + ' written - refresh page to close');
response.end();
break;
default:
Expand Down
10 changes: 5 additions & 5 deletions demo/exception.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// Example - generation of NodeReport on uncaught exception
require('nodereport').setEvents("exception");
var http = require("http");
require('nodereport').setEvents('exception');
var http = require('http');

var count = 0;

function my_listener(request, response) {
switch(count++) {
case 0:
response.writeHead(200,{"Content-Type": "text/plain"});
response.write("\nRunning NodeReport exception demo... refresh page to cause exception (application will terminate)");
response.writeHead(200, {'Content-Type': 'text/plain'});
response.write('\nRunning NodeReport exception demo... refresh page to cause exception (application will terminate)');
response.end();
break;
default:
Expand All @@ -18,7 +18,7 @@ function my_listener(request, response) {

function UserException(message) {
this.message = message;
this.name = "UserException";
this.name = 'UserException';
}

var http_server = http.createServer(my_listener);
Expand Down
8 changes: 5 additions & 3 deletions demo/fatalerror.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// Example - generation of Nodereport on fatal error (Javascript heap OOM)
require('nodereport').setEvents("fatalerror");
require('nodereport').setEvents('fatalerror');
var http = require('http');

var count = 0;

function my_listener(request, response) {
switch(count++) {
case 0:
response.writeHead(200,{"Content-Type": "text/plain"});
response.write("\nRunning NodeReport fatal error demo... refresh page to trigger excessive memory usage (application will terminate)");
response.writeHead(200, {'Content-Type': 'text/plain'});
response.write('\nRunning NodeReport fatal error demo... refresh page to trigger excessive memory usage (application will terminate)');
response.end();
break;
case 1:
Expand All @@ -17,8 +17,10 @@ function my_listener(request, response) {
while (true) {
list.push(new MyRecord());
}
/*eslint-disable no-unreachable */
response.end();
break;
/*eslint-enable no-unreachable */
}
}

Expand Down
23 changes: 12 additions & 11 deletions demo/loop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,35 @@
// Example - geneation of Nodereport via signal for a looping application
require('nodereport').setEvents("signal");
var http = require("http");
require('nodereport').setEvents('signal');
var http = require('http');

var count = 0;

function my_listener(request, response) {
switch(count++) {
case 0:
response.writeHead(200,{"Content-Type": "text/plain"});
response.write("\nRunning NodeReport looping application demo. Node process ID = " + process.pid);
response.write("\n\nRefresh page to enter loop, then use 'kill -USR2 " + process.pid + "' to trigger NodeReport");
response.writeHead(200, {'Content-Type': 'text/plain'});
response.write('\nRunning NodeReport looping application demo. Node process ID = ' + process.pid);
response.write('\n\nRefresh page to enter loop, then use \'kill -USR2 ' + process.pid + '\' to trigger NodeReport');
response.end();
break;
case 1:
console.log("loop.js: going to loop now, use 'kill -USR2 " + process.pid + "' to trigger NodeReport");
console.log('loop.js: going to loop now, use \'kill -USR2 ' + process.pid + '\' to trigger NodeReport');
var list = [];
var j='';
for (var i=0; i<10000000000; i++) {
for (var j=0; i<1000; i++) {
for (j=0; i<1000; i++) {
list.push(new MyRecord());
}
for (var j=0; i<1000; i++) {
for (j=0; i<1000; i++) {
list[j].id += 1;
list[j].account += 2;
}
for (var j=0; i<1000; i++) {
for (j=0; i<1000; i++) {
list.pop();
}
}
response.writeHead(200,{"Content-Type": "text/plain"});
response.write("\nNodeReport demo.... finished looping");
response.writeHead(200, {'Content-Type': 'text/plain'});
response.write('\nNodeReport demo.... finished looping');
response.end();
break;
default:
Expand Down
7 changes: 5 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@
"Richard Chamberlain <richard_chamberlain@uk.ibm.com> (https://github.com/rnchamberlain)"
],
"scripts": {
"test": "tap test/test*.js"
"test": "npm run lint && npm run tap",
"tap": "tap --timeout 10 test/test*.js",
"lint": "eslint **/*.js"
},
"bugs": {
"url": "https://github.com/nodejs/nodereport/issues"
},
"devDependencies": {
"tap": "^8.0.0"
"tap": "^8.0.0",
"eslint": "^3.13.1"
}
}
4 changes: 2 additions & 2 deletions test/common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ exports.validate = (t, report, options) => {
if (this.isWindows()) {
// On Windows we need to strip double quotes from the command line in
// the report, and escape backslashes in the regex comparison string.
t.match(nodeReportSection.replace(/"/g,''),
t.match(nodeReportSection.replace(/"/g, ''),
new RegExp('Command line: '
+ (options.commandline).replace(/\\/g,'\\\\')),
+ (options.commandline).replace(/\\/g, '\\\\')),
'Checking report contains expected command line');
} else {
t.match(nodeReportSection,
Expand Down
4 changes: 2 additions & 2 deletions test/test-exception.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
if (process.argv[2] === 'child') {
require('../');

function myException(request, response) {
function myException() {
const m = '*** test-exception.js: throwing uncaught Error';
throw new Error(m);
}
Expand All @@ -26,7 +26,7 @@ if (process.argv[2] === 'child') {
tap.plan(4);
// Verify exit code. Note that behaviour changed in V8 v5.4
const v8_version = (process.versions.v8).match(/\d+/g);
if (v8_version[0] < 5 || (v8_version[0] == 5 && v8_version[1] < 4)) {
if (v8_version[0] < 5 || (v8_version[0] === 5 && v8_version[1] < 4)) {
tap.equal(code, 0, 'Check for expected process exit code');
} else {
tap.equal(code, 1, 'Check for expected process exit code');
Expand Down