Dash.js: Remove `grunt-githooks` and use `precommit` script in `package.json`

[ProLoser]

I noticed this project uses grunt-githooks in order to enforce calling grunt lint as a pre-commit hook. It's unnecessary to use grunt-githooks for this purpose.

This code will accomplish the same task and would allow this project to reduce superfluous dependencies, especially considering the package was last updated 4 years ago.

// package.json
{
  // ...
  "scripts": {
    "precommit": "grunt lint",
    // ...
  }
  // ...
}

In my opinion, it would also be wise to add a new hook:

"preversion": "npm test"

After more probing, I think the way grunt-githooks is configured is either broken or confusing. Although the githook is called pre-commit it appears to be explicitly getting called in package.json anyway in prepublish

[mlasak]

Thank you for mentioning this. It is true that the outdated grunt-githooks dependency should be replaced. However, your suggested precommit "script" does not appear to be supported by npm. Which module are you recommending for this?

The functionality (lint before commit) should not get lost.

[ProLoser]

Oh man, you are right and I'm wrong. I completely forget my team uses https://github.com/typicode/husky which voids my argument for fewer dependencies 😅

Feel free to close this issue.

[mlasak]

We plan to remove the 'grunt-githooks' module. As you pointed out it is very out dated.
I am not decided yet if to include husky as new dep or to go for a more lightweight approach.

Thanks again

[ProLoser]

I will say 1 thing: at work, I put lint checks behind git push instead of git commit. I feel it gives a better compromise between disrupting productivity and protecting the code quality. This way people can save there work locally without being nitpicked about potentially WIP code, but still prevents them from pushing the quality regressions to the server. I also felt it made more sense for the average frequency of git push to git commit.

[mlasak]

This makes fully sense to me.