-
Notifications
You must be signed in to change notification settings - Fork 8
chore: add precommit hook and update linted files #1534
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
base: dev
Are you sure you want to change the base?
Conversation
Storybook deployed at https://unity-uds-staging.s3.us-west-2.amazonaws.com/pr-1534/index.html |
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 was thinking this hook would be a pre-push hook. (can we include tests too? it will save us from pushing a failed build to jenkins)
Sometimes while developing locally I just want to commit my changes. I may have code I know is not ready for a PR but I need to make a checkpoint.
We should as much as possible make sure all paths are covered.
git commit
calls the hook
yarn commit
or calling cz
does not call prepare-commit-msg
.husky/prepare-commit-msg
Outdated
exit 1 | ||
else | ||
echo "Adding linted files to staging..." | ||
git add . |
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.
We do not want to add all files, in an automatic way without the contributor knowing. This should be the developers choice to include changes.
- It is possible for a contributor to have changes they do not want to include with the commit.
- I think this prevents contributor from making smaller commits to group similar changes.
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.
So should this be a prepush hook instead? If it's a prepush hook, then commitizen shouldn't be called at all because that's already been done before they ran git push, right?
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.
Correct
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.
OK made that change
add precommit hook and update linted files
Checklist
Links