| ▲ | sethops1 7 hours ago |
| > 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. |
|
| ▲ | jayd16 5 hours ago | parent | next [-] |
| 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 3 hours ago | parent | next [-] | | > 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 3 hours ago | parent | prev | next [-] | | 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 3 hours ago | parent [-] | | 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." |
| |
| ▲ | krzyk 2 hours ago | parent | prev [-] | | Local hooks are just a convenience.
CI checks are assurances, you have to have them. If one hates the round-trip he/she will adopt hooks quickly. | | |
| ▲ | 2 hours ago | parent | next [-] | | [deleted] | |
| ▲ | jayd16 2 hours ago | parent | prev [-] | | LFS hooks are not just a convenience, for example. CI, despite being a useful thing in its own right, is not a replacement. |
|
|
|
| ▲ | baq 7 hours ago | parent | prev | next [-] |
| 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. |
| |
| ▲ | sgarland 6 hours ago | parent | next [-] | | 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. | |
| ▲ | skydhash 7 hours ago | parent | prev [-] | | 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. |
|
|
| ▲ | ufo 7 hours ago | parent | prev | next [-] |
| 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 6 hours ago | parent | next [-] | | 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 4 hours ago | parent [-] | | 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 4 hours ago | parent [-] | | 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 4 hours ago | parent [-] | | 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 6 hours ago | parent | prev [-] | | You can issue installation instructions on linter failure in CI. |
|
|
| ▲ | IshKebab 7 hours ago | parent | prev | next [-] |
| 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 7 hours ago | parent [-] | | 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 6 hours ago | parent [-] | | 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. | | |
| ▲ | jayd16 2 hours ago | parent | next [-] | | 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. | |
| ▲ | IshKebab 5 hours ago | parent | prev [-] | | 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. |
|
|
|
|
| ▲ | locknitpicker 3 hours ago | parent | prev [-] |
| > 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. |