documentation icon indicating copy to clipboard operation
documentation copied to clipboard

Add pre-commit hooks

Open Strift opened this issue 1 year ago • 6 comments

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)

Strift avatar Feb 19 '24 16:02 Strift

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.

guimachiavelli avatar Mar 20 '24 16:03 guimachiavelli

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 😅 We would rather have a contribution with linting issue (we can update ourselves) rather than no contribution at all. The linting problem should be visible in the CI associated to the PR. It's already the case right?

But only my opinion, I had bad experiences with pre-commit hook when trying to contribute to other opensource repositories.

curquiza avatar Mar 20 '24 16:03 curquiza

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

curquiza avatar Mar 20 '24 16:03 curquiza

You have infinitely more experience in managing public repositories, @curqui, so I'm inclined to side with you on this matter. I do empathize with @Strift, though, as I have uh occasionally forgotten to run linters.

guimachiavelli avatar Mar 21 '24 18:03 guimachiavelli

Even if I have more experience, I don't have THE experience 😊 IMO, the most important is you also feel comfortable with your tool. So, the best world is to be able to customize pre-commit hook on your sides only, and don't impose it on contributors.

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.

curquiza avatar Mar 25 '24 13:03 curquiza

I think it's fine to have it in CI only for now :)

Strift avatar Mar 25 '24 13:03 Strift