-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
tests: use husky to run tests before commits and pushes #11111
Conversation
PS: I did not add the changed package-lock.json file but we may have to update it. Also see #11046 |
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.
That's nice 🐶
Because a common rule in Git is that the code should always run, I guess we can use precommit
. And prepush
does not harm :).
I seen #11046. I think I'll take care of adding it after manually upgraded all our dependencies to prepare v6.5. There's no benefit in adding it if we're not already up-to-date. |
I just noticed this PR is number #11111 😄 |
This will probably not work on most Windows machines @ncoden, why not ouput it directly with Node.js? |
Ah, yeah... |
Actually, what would not run on Windows ? The whole NPM environement with npm scripts is based on bash, and Windows users are invited to set by themselve the program to use for commands. Also, using JS scripts does not resolve the compatibility issue as we have to run |
bash can not be used in cmd and git and npm are globally available on the path but bash isn't, just in the extra environment. Will test later on my machine but it will not run afaik. |
I'll provide a working solution. |
There you go =) |
Thanks 👍 |
Oops, small copy paste error =/ |
Fixed :) |
Actually had it prepared here but not commited anymore. |
Also, the Sass tests output is not colored anymore. Do you have an idea of what is causing this ? |
I guess this is because it is running in another context now and the color support is disabled there. I'm checking if we can use another solution here. |
Should be colored again now. |
Forcing the colors mode now by default. For sass-lint we might move to stylelint somewhere in the future. |
I guesset I should set the setting to rebase by default here too. Should I squash merge or rebase now and push force the last 3 commits as one? |
Please always put the context in your commits. "chore: remove code test output in husky scripts" 😉 |
Since no reviews are based on these commits, yes please do shash them ;). |
Side note: I really love this emoji 🐶 |
I guess it's good to go 👍 |
…v6.5.0 d9d307e tests: use husky to run tests before commits and pushes eaeebbf chore: add message for about Husky at precommit & prepush c353e26 chore: use Node.js for precommit and prepush output 38f97aa chore: change prepush script to use the new script adfbb8f chore: update package-lock.json e6e8614 fix: fix wrong script for husky prepush hook 6fcaa0c fix: force color mode of mocha, remove code test output in husky scripts 073d0f2 fix: force color mode for gulp and husky scripts Co-Authored-By: Nicolas Coden <nicolas@ncoden.fr> Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>
We should force to run
npm test
on each commit / push.Should we use
precommit
andprepush
or just one of both?Closes #11042
The rest of it will be probably covered in future code quality related PRs for all devs.