-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
src: implement standard.js linting #1794
Conversation
I've no real preferences re. Since we have Travis enabled now and the linting is run as part of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RSLGTM. Maybe add 'use strict';
to all files while you're here?
ce9d0b4
to
e9fefd9
Compare
implemented 'use strict' and a couple of other minor tweaks including making consistent spacing with array declarations |
In addition: * moved module.exports to the bottom * no single-line if statements * no if statements without a { * const for requires * array declarations get spaces inside [ ] * 'use strict' in all .js files PR-URL: #1794 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: João Reis <reis@janeasystems.com>
@rvagg it would be good to get this one landed to avoid it becoming outdated. AFAICT this is ready to land and I can do it tomorrow, please let me know if I shouldn't. |
IMO it's time for some proper linting, the code style is all over the place and the existing rules were super minimal. Current style is pretty close to standard.js, and it's a fairly universally accepted and understood style.
I've done a couple of additional things on top:
module.exports
to the bottomif
statementsif
statements without a{
const
for requiresThis is going to make it harder to manage older pull requests of course, so this comes with a cost.
Thoughts?