cannawen / metric_units_reddit_bot

Reddit bot converting imperial units to metric units
https://www.reddit.com/user/metric_units
GNU General Public License v3.0
78 stars 34 forks source link

Add lint-staged and husky #97

Closed sirugh closed 6 years ago

sirugh commented 6 years ago

PR Checklist

cannawen commented 6 years ago

Thank you for the PR!! Husky looks great :)

I'm a bit worried of adding the "eslint --fix" into the project without going through and immediately fixing all the issues though (#60), because there might be files failing the lint checks and then whoever touches the file next will be forced to fix it. I'm not sure if this a valid is a concern though, will the link checks be generally easy/quick fixes that "anyone" should know how to do? Or we can document somewhere that they can use --no-verify if they don't know how to fix a lint issue (this is only for when #60 is not merged yet. After #60 is merged, I am ok with forcing people to pass the lint check, since it will only be their own code they are touching)

But I am very excited to get eslint in... this inconsistent formatting has been driving me crazy

sirugh commented 6 years ago

eslint --fix is actually great for the trivial things like ensuring semicolons or adding spaces between function names and parens. You'll probably want to run the entire project through the linter as well to ensure forward compliance as you pointed out. If you have concerns about contributors not knowing how to fix a linting issue I would recommend putting a linting section in your contributing docs that links to the eslint rules.

As for lint errors/warnings, you can customize the levels to your desired outcome. For example, you probably want the mocha .only to error, but an unnamed function can just be a warning.

cannawen commented 6 years ago

I checked out your branch, did an npm install, but was still able to commit failing tests, console logs, and "only" statements. Any ideas what is going wrong? I just did a git add -am "message" without --no-verify ... I got rid of all files in the ./.git/hooks directory so there's no interference.

Also: the es-lint didn't fix any formatting... very strange

Link to branch: https://github.com/cannawen/metric_units_reddit_bot/commits/sirugh-precommit

sirugh commented 6 years ago

Did you see husky run at all? May be related to existing hooks.

sirugh commented 6 years ago

Done.

cannawen commented 6 years ago

Hey @sirugh can you do a rebase or merge off master? There are conflicts :( I promise I'll merge your PR in next so you don't need to deal with any more conflicts :P

sirugh commented 6 years ago

@cannawen rebased!