4 ms·

> what workflows have worked best for you to get everyone to run the hooks By running the linters and any other checks on CI instead.

baq 6mo ago
autoformatter and autofix linter results can be committed and pushed by CI into the PR branch itself. this is a pain sometimes, but as a repo owner it should protect your sanity.
- skydhash 6mo ago
  I just add a check workflow that test that the files are well formatted and linted. If it passes, one of the key things I check are changes to the configuration. Some tools allows for bypass comments, so I keep an eye out for those too.
- sgarland 6mo ago
  Yep. Nothing I hate more than some trivial formatting error that could easily fix itself halting CI. I am all for consistent formatting and linting, I just think it should be silently handled without fuss.
IshKebab 6mo ago
As well, not instead. Just add `pre-commit run -a` to your CI. Job done. It's still annoying for new contributors though because they might not know how to set up pre-commit (which was quite a pain until recently because it's written in Python).
- Flimm 6mo ago
  To clear up any confusion, Git runs pre-commit hooks, and they can be written in any programming language. There's a completely separate and independent project that gave itself the confusing "pre-commit" name, and it is written in Python. This project aims to make it easier to configure pre-commit hooks. An alternative to it is "prek", written in Rust.
  - sgarland 6mo ago
    Yes, and I hate it so, so much, and frankly don’t get the appeal. You want one-click installation of hooks? Bundle a shell script called run_first.sh that symlinks the hooks into .git.
    - IshKebab 6mo ago
      The pre-commit tool does way more than that. For example the clang-format hook will download and run a specific version of clang-format.
    - jayd16 6mo ago
      That doesn't automate the maintenance of the hooks, doesn't handle cross branch differences, opens you up to all kinds of security holes because now it'll just do whatever the hook points to which is likely to be in the repo and not in some special higher scrutiny flow... All the push back to making this system good just ensures its as terrible as the nay-sayers fear.
ufo 6mo ago
We do run the linter on CI as well, but I think our comitters would get faster feedback if they ran those checks locally.
- lou1306 6mo ago
  Well you can tell them to please enable hooks in the PR guidelines, but you cannot really police what they do or don't run on their own machines.
  - jayd16 6mo ago
    This is very much not a serious solution. Look at the case of LFS. LFS needs an install step and it needed to be brought into git itself to cut through all of the problems. Manually managing hooks is not sufficient. No amount of "please don't fuck it up" in the readme is going to save you. Even CI checks for what should and shouldn't look like an lfs stub is non-trivial. I don't think such a thing even exists today.
    - lou1306 6mo ago
      The alternative is have hooks _forcibly_ run on people's machines, which is fantastic as an attack vector and CVE generator but probably not a good choice in other respects.
      - jayd16 6mo ago
        No there are a million miles in-between no support/Don't use it and arbitrary code execution. Signed git plugins and manifest or a canonical way to define hooks in repo that most tools can interface with and allow the user to automatically set up but asks to do so or really so much more. I don't know why people get fixated on this as if 99.999% of what git pulls down isn't code you expect to run and there are systems in place to protect that.
- esafak 6mo ago
  You can issue installation instructions on linter failure in CI.
jayd16 6mo ago
Why waste a round trip, build time, loss of flow and CI machine queue wait time when you can catch things early? CI should also run all the checks but CI checks are not a replacement for local hooks. LFS and things like it can't be implemented as remote CI checks. Why are we acting like a James Bond villain, slowly lowering the changes into the vat of sharks after we've left the room? I want the hooks. Can we talk about making that easy, assuming some people want them?
- n_e 6mo ago
  > Why waste a round trip, build time, loss of flow and CI machine queue wait time when you can catch things early? Because we want to be sure that the checks have passed, and that they have passed in a clean environment. Contributors can, in addition, use git hooks, or run tests in watch mode, or use their IDE. Also it's annoying to have slow git hooks if you commit often.
- crote 6mo ago
  You're looking for a technological solution for a human problem. Automatically running arbitrary code from random repositories is a Really Bad Idea, so Git will almost certainly never auto-install pre-commit hooks. Just mention it in the README and run a checker in CI to confirm they are using it, it really isn't that difficult. People wasting 2 minutes of their own time once during their first contribution because they didn't read the README is not that big of a deal. What's next, you want a script to automatically sign a project's legally-binding CLA on checkout?
  - jayd16 6mo ago
    You're talking out of both sides of your face here. It's dangerous and also it's super easy and you should do it first thing without having to think because it's so easy. You shouldn't run this code but also the build machine automatically runs it. We already know we're definitely going to run some of these. We know we want to maintain changes to these hooks. Can we stop pretending like we're not doing that? We get it. Some of these will be untrusted so let's design a system to handle that instead of not designing a system and deciding to be just short of as unsafe as possible. Automation an uniformity increases safety. Human intervention increases human error. Its just a matter of actually finding a good solution to know what is trusted but instead we get "just set it up manually because its safer."
locknitpicker 6mo ago
> By running the linters and any other checks on CI instead. Running linters on CI is an antipattern if there was ever one. That and configuring pipeline runs to fail for linting issues. Sometimes some people just want to create their own problems. Configuring the editor solves most of the problems, and hooks add a failsafe. Once the code is committed, it should be immutable.

baq 6mo ago

autoformatter and autofix linter results can be committed and pushed by CI into the PR branch itself. this is a pain sometimes, but as a repo owner it should protect your sanity.

skydhash 6mo ago
I just add a check workflow that test that the files are well formatted and linted. If it passes, one of the key things I check are changes to the configuration. Some tools allows for bypass comments, so I keep an eye out for those too.
sgarland 6mo ago
Yep. Nothing I hate more than some trivial formatting error that could easily fix itself halting CI. I am all for consistent formatting and linting, I just think it should be silently handled without fuss.

IshKebab 6mo ago

As well, not instead. Just add `pre-commit run -a` to your CI. Job done. It's still annoying for new contributors though because they might not know how to set up pre-commit (which was quite a pain until recently because it's written in Python).

Flimm 6mo ago
To clear up any confusion, Git runs pre-commit hooks, and they can be written in any programming language. There's a completely separate and independent project that gave itself the confusing "pre-commit" name, and it is written in Python. This project aims to make it easier to configure pre-commit hooks. An alternative to it is "prek", written in Rust.
- sgarland 6mo ago
  Yes, and I hate it so, so much, and frankly don’t get the appeal. You want one-click installation of hooks? Bundle a shell script called run_first.sh that symlinks the hooks into .git.
  - IshKebab 6mo ago
    The pre-commit tool does way more than that. For example the clang-format hook will download and run a specific version of clang-format.
  - jayd16 6mo ago
    That doesn't automate the maintenance of the hooks, doesn't handle cross branch differences, opens you up to all kinds of security holes because now it'll just do whatever the hook points to which is likely to be in the repo and not in some special higher scrutiny flow... All the push back to making this system good just ensures its as terrible as the nay-sayers fear.

ufo 6mo ago

We do run the linter on CI as well, but I think our comitters would get faster feedback if they ran those checks locally.

lou1306 6mo ago
Well you can tell them to please enable hooks in the PR guidelines, but you cannot really police what they do or don't run on their own machines.
- jayd16 6mo ago
  This is very much not a serious solution. Look at the case of LFS. LFS needs an install step and it needed to be brought into git itself to cut through all of the problems. Manually managing hooks is not sufficient. No amount of "please don't fuck it up" in the readme is going to save you. Even CI checks for what should and shouldn't look like an lfs stub is non-trivial. I don't think such a thing even exists today.
  - lou1306 6mo ago
    The alternative is have hooks _forcibly_ run on people's machines, which is fantastic as an attack vector and CVE generator but probably not a good choice in other respects.
    - jayd16 6mo ago
      No there are a million miles in-between no support/Don't use it and arbitrary code execution. Signed git plugins and manifest or a canonical way to define hooks in repo that most tools can interface with and allow the user to automatically set up but asks to do so or really so much more. I don't know why people get fixated on this as if 99.999% of what git pulls down isn't code you expect to run and there are systems in place to protect that.
esafak 6mo ago
You can issue installation instructions on linter failure in CI.

jayd16 6mo ago

Why waste a round trip, build time, loss of flow and CI machine queue wait time when you can catch things early? CI should also run all the checks but CI checks are not a replacement for local hooks. LFS and things like it can't be implemented as remote CI checks. Why are we acting like a James Bond villain, slowly lowering the changes into the vat of sharks after we've left the room? I want the hooks. Can we talk about making that easy, assuming some people want them?

n_e 6mo ago
> Why waste a round trip, build time, loss of flow and CI machine queue wait time when you can catch things early? Because we want to be sure that the checks have passed, and that they have passed in a clean environment. Contributors can, in addition, use git hooks, or run tests in watch mode, or use their IDE. Also it's annoying to have slow git hooks if you commit often.
crote 6mo ago
You're looking for a technological solution for a human problem. Automatically running arbitrary code from random repositories is a Really Bad Idea, so Git will almost certainly never auto-install pre-commit hooks. Just mention it in the README and run a checker in CI to confirm they are using it, it really isn't that difficult. People wasting 2 minutes of their own time once during their first contribution because they didn't read the README is not that big of a deal. What's next, you want a script to automatically sign a project's legally-binding CLA on checkout?
- jayd16 6mo ago
  You're talking out of both sides of your face here. It's dangerous and also it's super easy and you should do it first thing without having to think because it's so easy. You shouldn't run this code but also the build machine automatically runs it. We already know we're definitely going to run some of these. We know we want to maintain changes to these hooks. Can we stop pretending like we're not doing that? We get it. Some of these will be untrusted so let's design a system to handle that instead of not designing a system and deciding to be just short of as unsafe as possible. Automation an uniformity increases safety. Human intervention increases human error. Its just a matter of actually finding a good solution to know what is trusted but instead we get "just set it up manually because its safer."

locknitpicker 6mo ago

> By running the linters and any other checks on CI instead. Running linters on CI is an antipattern if there was ever one. That and configuring pipeline runs to fail for linting issues. Sometimes some people just want to create their own problems. Configuring the editor solves most of the problems, and hooks add a failsafe. Once the code is committed, it should be immutable.