-
Notifications
You must be signed in to change notification settings - Fork 2
STIJ-195: Added pre-commit hook support. #170
base: develop
Are you sure you want to change the base?
Conversation
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'll add Peter to review this as well
postinstall.sh
Outdated
cp -R ./node_modules/jquery ./public/styleguide/vendor/jquery; | ||
cp -R ./node_modules/chosen-js ./public/styleguide/vendor/chosen-js; | ||
|
||
cp ./scripts/pre-commit ./.git/hooks/pre-commit |
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 prefer something more clear for the git-hooks e.g. a .git-hooks folder which is also hidden on the filesystem by default.
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.
Use symlink instead of copy.
No support for windows users?
scripts/pre-commit
Outdated
git stash -q --keep-index | ||
|
||
# Test prospective commit | ||
gulp validate; |
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 prefer something that uses the package manager as wrapper to have uniformity. E.g. npm pre-commit. For PHP projects we could use composer pre-commit.
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.
No longer relevant?
Description
Added a pre-commit hook integration to automatically test things like JS and SASS validation.
Motivation and Context
This might reduce errors in the production code.
How Has This Been Tested?
Did a local commit test.
Screenshots (if appropriate):
Types of changes
Checklist: