-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add eslint config, run it in ci for bot.js #171
Conversation
The entire bot is basically built on runtime requiring of dynamically determined strings.
In the future this should lint all javascript, but this is what we've got for now.
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.
This is exactly what I set up locally. eslint barfs over most files with airbnb here, but this is better than nothing.
I'm not familiar with Travis, but if it passes eslint isn't working right ;). |
"mocha": "^2.5.3", | ||
"sinon": "^2.3.6" | ||
}, | ||
"scripts": { | ||
"start": "node --max_old_space_size=350 index.js", | ||
"install": "./installModules.sh", | ||
"test": "mocha $(git ls-files | grep -E \"test.js$\")" | ||
"test": "mocha $(git ls-files | grep -E \"test.js$\")", | ||
"lint": "eslint bot.js" |
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.
Just saw this.
eslint --ignore-pattern node_modules *.js modules/**/*.js
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.
I'd like to move to that, but I'd rather have it run on the files it passes on for now and add the remaining files later.
Incrementally adding which files it lints allows us to get some signal from travis for some changes without having to fixup everything yet (which I'm not up to at the moment).
There's still a long way to go before we can run this on the whole thing, but this is a start.
@brhoades, want to take a look?