Add husky pre-commit hook #314
Conversation
package.json
Outdated
| "prepare": "husky install" | ||
| }, | ||
| "lint-staged": { | ||
| "{src,test}/**/*.ts": [ "tslint --fix" ] |
There was a problem hiding this comment.
[F] Lifted from typicode/husky#949 (comment)
There was a problem hiding this comment.
makes sense, nice hook to get this into everyones commit flow.
mattseddon
force-pushed
the
add-husky-precommit-hook
branch
from
c525c7e
to
4b4c4bf
Compare
| @@ -22,8 +22,8 @@ | |||
| "forwardPorts": [3000], | |||
|
|
|||
| // Specifies a command that should be run after the container has been created. | |||
| "postCreateCommand": "npm install", | |||
| "postCreateCommand": "npm install && npm run prepare", | |||
There was a problem hiding this comment.
[F] For v6 of husky we have to run an extra script. Husky dropped support for autoinstall when they moved to v6. Reasoning is detailed here.
|
I have managed to get the tests running headless from the command line inside the container using the recommendations for setting up a Gitlab CI. I am pretty sure that I could get test running in the pre-commit hook for both container and non-container dev. @ryanluker WDYT? Is there enough value in me adding the test suite into the pre-commit hook? |
| @@ -22,8 +22,8 @@ | |||
| "forwardPorts": [3000], | |||
|
|
|||
| // Specifies a command that should be run after the container has been created. | |||
| "postCreateCommand": "npm install", | |||
| "postCreateCommand": "npm install && npm run prepare", | |||
package.json
Outdated
| "prepare": "husky install" | ||
| }, | ||
| "lint-staged": { | ||
| "{src,test}/**/*.ts": [ "tslint --fix" ] |
There was a problem hiding this comment.
makes sense, nice hook to get this into everyones commit flow.
package.json
Outdated
| @@ -264,6 +269,8 @@ | |||
| "@types/vscode": "1.27.0", | |||
| "chai": "4.2.0", | |||
| "cross-env": "5.2.0", | |||
| "husky": "^6.0.0", | |||
| "lint-staged": "^11.0.0", | |||
There was a problem hiding this comment.
super minor: pin these dependencies to avoid changes (similar to the others).
I was also consider unpinning these (using the ^) now that the CICD system is working nicely 🤔 .
|
@mattseddon Thanks for the PR on the pre-commit hook! https://code.visualstudio.com/api/working-with-extensions/continuous-integration#gitlab-ci |
@ryanluker no worries, I am happy to leave tests out of the hook. I have pinned the new dependencies as requested. I'll look into fixing up the linux test suite (devcontainer / CI/CD pipeline) next weekend 👍🏻. |
|
@mattseddon Good discussion, thanks a ton for the contributions! |
|
@mattseddon Just used husky locally for a PR I am working on went well! |
😢 that's always the way with adding new dependencies. Sorry. |
|
@mattseddon haha no need to feel sorry, I have had this happen at work a bit when adding new pre-commit type stuff so I was just wondering if it was expected. |
This PR adds a Husky as promised here.
The hook runs
tslint --fixagainst all staged files before allowing a commit to be processed (iftslintcannot fix the issues in the staged file then the commit will not proceed).I initially could not get tests to run from the command line inside of the
devcontainer. There are further changes required to get that part of the hook to run and I felt that stopping here and asking the question as to whether or not it is desirable to run the tests in the pre-commit hook would be the best way to proceed.I looked into the
contributing.mdand did not see any need to mention the new hook. I have however added the prepare script to the devcontainer so that the hooks are installed after it is built. Again, happy to change if you see things differently 👍🏻.LMK if you would like to see any changes or additions to this PR 👍🏻. Happy to raise a follow up to add the tests in, if desired.