-
Notifications
You must be signed in to change notification settings - Fork 6
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
Additional ci setup #16
Conversation
Have not added tests yet; refactoring is needed
Including commenting out some apparently unneeded dev dependencies
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.
Looks good!! Just made a small comment below in your package.json.
} | ||
}, | ||
"lint-staged": { | ||
"*.js": ["eslint"] |
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.
Could add --fix to auto fix errors on commit. Does lint-staged need to include jsx files too?
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.
Thanks! The reason I don't the linter running over jsx
files is that I don't have any jsx
files. I read that Facebook doesn't recommend using the jsx
extension anymore (facebook/create-react-app#87) so I've just been using js
for everything. On the other hand, AirBnB recommends the jsx
extension, so it could go either way. http://airbnb.io/javascript/react/
Similarly, I don't run --fix
because I've got a few areas where eslint throws a warning (but not an error) that I don't want to change that could be changed by --fix
. Thanks very much for the thoughtful comments :)
Add additional CI, including Husky.
This PR also adds CI-related badges to the README