search

cursorless-dev / cursorless

Don't let the cursor slow you down
https://www.cursorless.org/
MIT License
1.09k stars 77 forks source link

Don't install pre-commit hooks in nix flake #2461

Open pokey opened 4 days ago

pokey commented 4 days ago

Why do we install pre-commit hooks in our nix flake? I would be strongly inclined to remove that step, but maybe there's some reason we have to do this as setting up the nix flake? Totally missed that that was what was happening in #1908

cc/ @auscompgeek @fidgetingbits

fidgetingbits commented 4 days ago

It's not required no. Since it's a development shell, and I assume people using it would be committing things, it made sense to install the hooks? I don't mind removing it, but also curious why you think it's bad? Does it clobber existing tools if you already had them and then test the dev shell or something?

Lots of flakes do it afaik, but also would be ok to just print a message in the shellHook saying it pre-commit install can be run if needed?

pokey commented 4 days ago

Just feels a bit invasive. I don't have them installed locally so was surprised when I couldn't commit something and couldn't figure out why. Turned out it was because I had tested out the flake PR

For me, setting up the flake shouldn't have any impact on non-flake setup. I had thought isolation was one of the goals

fidgetingbits commented 3 days ago

Ah interesting, that makes sense. Bad assumption on my part. I'll send a PR to remove it.

Why don't you use pre-commit locally out of curiosity. I specifically wonder about cases where it would catch stuff that can't be auto fixed by running it in CI?

pokey commented 3 days ago

I prefer my git commit to be dumb, fast, and predictable. It's plumbing