search

meilisearch / documentation

Meilisearch documentation
https://docs.meilisearch.com
MIT License
145 stars 238 forks source link

Add pre-commit hooks #2718

Open Strift opened 7 months ago

Strift commented 7 months ago

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)

guimachiavelli commented 6 months ago

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.

curquiza commented 6 months ago

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 commented 6 months ago

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

guimachiavelli commented 6 months ago

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.

curquiza commented 5 months ago

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.

Strift commented 5 months ago

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