tmpfs / async-validate

Asynchronous type validation for node and the browser
Other
314 stars 45 forks source link

JSHint and Style checking to keep code consistency #42

Closed ghost closed 8 years ago

ghost commented 8 years ago

On the thought we might have people contributing to this, I am wondering if it might be a good idea to introduce JsHinting and Style checking using JSCS on the code base.

tmpfs commented 8 years ago

I looked into this recently with eslint but deemed I didn't have the time right now.

Would be willing to look at this again a little later, I have a list of relatively important features at the moment ;)

ghost commented 8 years ago

I worked with JSCS recently so I can try to hook up the NPM scripts for this. :)

ghost commented 8 years ago

@freeformsystems I put in JSHINT and could run it as an NPM script. But I guessed there is a fair amount of refactoring that needs to be done. As this comes up to a preference of coding style I wish to consult you before proceeding.

These are the results of a sample run against the lib folder, I think they can be fixed easily but I am not sure if the changes might be against your coding style.

lib/iterator.js: line 3, col 5, Comma warnings can be turned off with 'laxcomma'.
lib/iterator.js: line 2, col 25, Bad line breaking before ','.
lib/iterator.js: line 24, col 14, Bad line breaking before ','.

lib/rule.js: line 2, col 3, Comma warnings can be turned off with 'laxcomma'.
lib/rule.js: line 1, col 30, Bad line breaking before ','.
lib/rule.js: line 2, col 35, Bad line breaking before ','.
lib/rule.js: line 33, col 7, Bad line breaking before ','.
lib/rule.js: line 34, col 7, Bad line breaking before ','.
lib/rule.js: line 35, col 7, Bad line breaking before ','.
lib/rule.js: line 36, col 53, Bad line breaking before ','.
lib/rule.js: line 37, col 7, Bad line breaking before ','.
lib/rule.js: line 62, col 7, Bad line breaking before '?'.
lib/rule.js: line 64, col 11, Bad line breaking before '||'.
lib/rule.js: line 68, col 5, Bad line breaking before '&&'.
lib/rule.js: line 69, col 5, Bad line breaking before '&&'.
lib/rule.js: line 74, col 5, Bad line breaking before '&&'.
lib/rule.js: line 116, col 5, Bad line breaking before '||'.
lib/rule.js: line 123, col 7, Bad line breaking before ','.
lib/rule.js: line 126, col 8, Unexpected use of '~'.

lib/schema.js: line 2, col 3, Comma warnings can be turned off with 'laxcomma'.
lib/schema.js: line 1, col 36, Bad line breaking before ','.
lib/schema.js: line 2, col 35, Bad line breaking before ','.
lib/schema.js: line 42, col 27, 'messages' is already defined.
lib/schema.js: line 69, col 7, Bad line breaking before ','.
lib/schema.js: line 70, col 7, Bad line breaking before ','.
lib/schema.js: line 71, col 7, Bad line breaking before ','.
lib/schema.js: line 72, col 17, Bad line breaking before ','.
lib/schema.js: line 73, col 33, Bad line breaking before ','.
lib/schema.js: line 75, col 57, Bad line breaking before ','.
lib/schema.js: line 129, col 17, Bad line breaking before ','.
lib/schema.js: line 130, col 9, Bad line breaking before ','.
lib/schema.js: line 131, col 9, Bad line breaking before ','.
lib/schema.js: line 161, col 7, Bad line breaking before '&&'.
lib/schema.js: line 170, col 11, Bad line breaking before ','.
lib/schema.js: line 184, col 35, Bad line breaking before ','.
lib/schema.js: line 185, col 43, Bad line breaking before ','.
lib/schema.js: line 186, col 37, Bad line breaking before ','.
lib/schema.js: line 193, col 15, Bad line breaking before '&&'.
lib/schema.js: line 197, col 15, Bad line breaking before '?'.
lib/schema.js: line 197, col 55, Bad line breaking before ','.
lib/schema.js: line 201, col 15, Bad line breaking before '?'.
lib/schema.js: line 202, col 17, Bad line breaking before '?'.
lib/schema.js: line 275, col 7, Bad line breaking before ','.
lib/schema.js: line 276, col 7, Bad line breaking before ','.
lib/schema.js: line 277, col 17, Bad line breaking before ','.
lib/schema.js: line 303, col 7, Bad line breaking before ','.
lib/schema.js: line 308, col 7, Bad line breaking before '||'.
lib/schema.js: line 69, col 7, 'value' is defined but never used.
lib/schema.js: line 70, col 7, 'rule' is defined but never used.
lib/schema.js: line 71, col 7, 'validator' is defined but never used.
lib/schema.js: line 196, col 17, 'values' is defined but never used.
lib/schema.js: line 248, col 26, 'result' is defined but never used.
tmpfs commented 8 years ago

Thanks, all the errors except:

lib/schema.js: line 69, col 7, 'value' is defined but never used.
lib/schema.js: line 70, col 7, 'rule' is defined but never used.
lib/schema.js: line 71, col 7, 'validator' is defined but never used.
lib/schema.js: line 196, col 17, 'values' is defined but never used.
lib/schema.js: line 248, col 26, 'result' is defined but never used.

Go against my style, if you can update the rules to omit all the other errors I'll take the PR.

Also, let's enforce single quotes, have that generate an error and I'll update the source files.

tmpfs commented 8 years ago

Oh, that one too:

lib/schema.js: line 42, col 27, 'messages' is already defined.

ghost commented 8 years ago

Ok sure I will break it into multiple commits. 1 PR to add a new npm script for lint for run JSHINT and JSCS, and update the package.json dev dependencies.

Then I will start jshinting and running jscs the individual the individual scripts so we can make the commits smaller. Single quotes has been depreciated in JSHINT and will be done by JSCS. I will be using the style used by Google + customized rules used by JSCS. The rules are configurable and I can adjust them to fit the styles once I run against each file and discussing with you.

The following is the initial config settings for JSHINT

{
    "node"     : true,
    "bitwise"  : true,
    "undef"    : true,
    "eqeqeq"   : true,
    "noarg"    : true,
    "mocha"    : true,
    "unused"   : true,
    "asi"      : true
}
tmpfs commented 8 years ago

Sure, sounds fine just remember to rebase before the PR ;)

tmpfs commented 8 years ago

Oh, I think lint is fine for the npm script name.

tmpfs commented 8 years ago

So I updated the jshint rules in b3f851e2968bbb2f8e902d6b5bfd42484c81a7ec and there are no lint errors but various jscs errors, do you have the time to resolve them against the current code style?