-
Notifications
You must be signed in to change notification settings - Fork 215
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add pre-commit hooks #2718
Comments
The docs team had this discussion a long time ago and I believe @curquiza was against running linters on actions because this might dissuade contributors from submitting PRs. |
Yeah I'm not a big fan of imposed pre-commit hook. It definitely can prevent people from submitting. It prevented me in the past for example 馃槄 But only my opinion, I had bad experiences with pre-commit hook when trying to contribute to other opensource repositories. |
Also, I think pre-commit hook can be set up individually (so you can set it up on your side @Strift) without imposing it to everyone |
Even if I have more experience, I don't have THE experience 馃槉 If not possible, I would go with imposing it on everyone if it really helps your day-to-day work. Your contributions are more than the external contributor's ones, and bring more impact and quality. It's important you feel comfortable with your work tools. |
I think it's fine to have it in CI only for now :) |
It often happens that PR are submitted (by me 馃ぁ) without running the linter first.
Adding a pre-commit hook to automatically run the linter before creating a commit can help with ensure that this is done and doesn't waste reviewers time.
(It can still be possible to commit bypass this protection if needed when doing quick & dirty stuff, the CI will still run for PRs)
The text was updated successfully, but these errors were encountered: