Skip to content
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

Remove husky dependency? #244

Open
rjuju opened this issue Dec 25, 2024 · 3 comments · May be fixed by #245
Open

Remove husky dependency? #244

rjuju opened this issue Dec 25, 2024 · 3 comments · May be fixed by #245

Comments

@rjuju
Copy link
Member

rjuju commented Dec 25, 2024

@pgiraud

I see that husky is creating a git pre-commit script and overriding developpers git configuration to call it automatically by stting core.hooksPath.

I'm not a fan of this approach for multiple reasons:

  • in general dependencies should have no business changing your git configuration behding your back
  • it also makes your life complicated if you want to configure your own hooks (for instance if you want to run ruff or anything else)
  • apparently the various npm checks are only checked in this pre-commit hook but not in the CI, which seems like a bad idea.

I think that we could entirely remove the husky dependency, which doesn't really solve any problem anymore, and instead provide some pre-commit script with some documentation so that users are free to configure it, and call that script in the CI.

@pgiraud
Copy link
Member

pgiraud commented Dec 27, 2024

I agree, I can do that.

@rjuju
Copy link
Member Author

rjuju commented Dec 28, 2024

thanks!

@pgiraud pgiraud linked a pull request Dec 30, 2024 that will close this issue
@pgiraud
Copy link
Member

pgiraud commented Dec 30, 2024

  • apparently the various npm checks are only checked in this pre-commit hook but not in the CI, which seems like a bad idea

This is wrong. npm checks are done in CI via https://github.com/powa-team/powa-web/blob/master/.github/workflows/javascript.yml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants