4 ms·

> Do you really want to push changes to some remote CI system only to be told it's failed some boring QA check? Yes, that's what CI/CD is for. > Pre-commit is

> Do you really want to push changes to some remote CI system only to be told it's failed some boring QA check? Yes, that's what CI/CD is for. > Pre-commit is completely optional for each developer. No it is not, it's installed at your git pre-commit hook, and it gets run every fucking time I commit, even if I've set up my editor correctly to auto-format and lint everything at I develop, meaning 99.9999% of the time sure the code I commit will pass all these linter checks. I can use git commit --no-verify to bypass pre-commit, but then again, what's the point of using pre-commit in the first place if you need to bypass it? There is absolutely no point of linting twice locally and thrice in total just to hit the first stage of your deployment.

chucksmash 4y ago
rm ./.git/hooks/pre-commit Or just do not install the hook in the first place. Choice is still the developer's unless your team is doing something else here that limits your ability to delete/rename files locally.
- wyuenho 4y ago
  That does not alleviate the problem with difficulty having your editor to use the correct version and settings for the linters and formatters. This approach also does not address the issue of having 2 separate sets of configuration when you are trying address problem number 1 above.
doliveira 4y ago
How is it mandatory? You can just not install the hooks
- wyuenho 4y ago
  1. Because it defeats the purpose of pre-commit. 2. pre-commit install installs the hook by default. 3. If you happen to fail linting a few times a year, there's always an anal coworker telling your boss in a 1-on-1 you are not following some BS ways of working.
  - doliveira 4y ago
    > 1. Because it defeats the purpose of pre-commit. No, it doesn't. Pre-commits are just a DX bonus so you won't have to wait for the CI/CD server. 2. pre-commit install installs the hook by default. Am I missing something? If you run "$ pre-commit install" of course it will install the hooks. Do you mean that poetry install, setup.py themselves are making the magic of installing the hooks themselves? If so, it's indeed a bad practice. 3. If you happen to fail linting a few times a year, there's always an anal coworker telling your boss in a 1-on-1 you are not following some BS ways of working. I don't get why anyone would care, unless you're polluting the CI/CD history with tons of tiny commits all day along just to run linting.
    - fnord123 4y ago
      Filling the CI/CD pipeline with tiny commits to fix linting when it should be done before commit or push is why pre-commit exists. It's not BS, clogging the job queue shits in everyone else. Of course I don't know GPs specific context so they may have other details, but generally it's
      - doliveira 4y ago
        Yeah, I agree. It's just that they can just run pre-commit manually if the hooks bother them that much
      - wyuenho 4y ago
        Except this problem rarely comes up. Everyone has their favorite way to set up their editors to lint and format automatically using the project's config. Over my career, I see that every company has some version of: 1. New guy joins a team, wants to push code ASAP 2. He sets up his favorite editor and punt configuring linters and formatters 3. Code pushed, CI fails at linting 4. Some well-meaning coworker or new guy suggests some variations of husky/pre-commit/fancy git hook scripts 5. Team agrees and put that into the repo 6. 6 months to 1 year later the entire team realizes its benefit does not out-weight the cost, and unanimously agree to rip it out unceremoniously.
globular-toast 4y ago
> Yes, that's what CI/CD is for. Not really. CI/CD is a methodology where a team immediately integrates new changes into a trunk/release. The CI/CD pipeline is there to give the team the confidence to merge your stuff. When I see people constantly pushing breaking code into a CI pipeline I see an incredible amount of wasted time and shared computing resource. Especially if it's some trivial formatting check that you should have already done locally. You do what works for you, but tools like pre-commit were invented to save time and effort and they work well. > I've set up my editor correctly to auto-format and lint everything at I develop, meaning 99.9999% of the time sure the code I commit will pass all these linter checks. Then what is the problem? Have your team installed hooks that take a long time to run? Even on larger codebases pre-commit adds a negligible amount of time to each commit, unless perhaps you've touched every file in the codebase or something. Honestly your gripe with pre-commit seems mostly irrational.
- wyuenho 4y ago
  > The CI/CD pipeline is there to give the team the confidence to merge your stuff. That's why linting is a part of every CI stage. Linters check your code for bugs. > Have your team installed hooks that take a long time to run? Yes, they are called tests.
  - globular-toast 4y ago
    Wait, you're not supposed to run tests with pre-commit. I can see why that would be frustrating. If that's the case, your team is doing it wrong.
    - wyuenho 4y ago
      Perhaps, but it's not just one team, and if you are doing code quality checks on CI anyway, I don't see the point of pre-commit (for most people).
Too 4y ago
Does it matter if it’s run twice? Usually it’s so fast you hardly even notice it. If your pre-commit takes more than 3 seconds to run it’s set up incorrectly IMO, and should belong to a more manually (and CI of course) invoked test suite instead.

chucksmash 4y ago

rm ./.git/hooks/pre-commit Or just do not install the hook in the first place. Choice is still the developer's unless your team is doing something else here that limits your ability to delete/rename files locally.

wyuenho 4y ago
That does not alleviate the problem with difficulty having your editor to use the correct version and settings for the linters and formatters. This approach also does not address the issue of having 2 separate sets of configuration when you are trying address problem number 1 above.

doliveira 4y ago

How is it mandatory? You can just not install the hooks

wyuenho 4y ago
1. Because it defeats the purpose of pre-commit. 2. pre-commit install installs the hook by default. 3. If you happen to fail linting a few times a year, there's always an anal coworker telling your boss in a 1-on-1 you are not following some BS ways of working.
- doliveira 4y ago
  > 1. Because it defeats the purpose of pre-commit. No, it doesn't. Pre-commits are just a DX bonus so you won't have to wait for the CI/CD server. 2. pre-commit install installs the hook by default. Am I missing something? If you run "$ pre-commit install" of course it will install the hooks. Do you mean that poetry install, setup.py themselves are making the magic of installing the hooks themselves? If so, it's indeed a bad practice. 3. If you happen to fail linting a few times a year, there's always an anal coworker telling your boss in a 1-on-1 you are not following some BS ways of working. I don't get why anyone would care, unless you're polluting the CI/CD history with tons of tiny commits all day along just to run linting.
  - fnord123 4y ago
    Filling the CI/CD pipeline with tiny commits to fix linting when it should be done before commit or push is why pre-commit exists. It's not BS, clogging the job queue shits in everyone else. Of course I don't know GPs specific context so they may have other details, but generally it's
    - doliveira 4y ago
      Yeah, I agree. It's just that they can just run pre-commit manually if the hooks bother them that much
    - wyuenho 4y ago
      Except this problem rarely comes up. Everyone has their favorite way to set up their editors to lint and format automatically using the project's config. Over my career, I see that every company has some version of: 1. New guy joins a team, wants to push code ASAP 2. He sets up his favorite editor and punt configuring linters and formatters 3. Code pushed, CI fails at linting 4. Some well-meaning coworker or new guy suggests some variations of husky/pre-commit/fancy git hook scripts 5. Team agrees and put that into the repo 6. 6 months to 1 year later the entire team realizes its benefit does not out-weight the cost, and unanimously agree to rip it out unceremoniously.

globular-toast 4y ago

> Yes, that's what CI/CD is for. Not really. CI/CD is a methodology where a team immediately integrates new changes into a trunk/release. The CI/CD pipeline is there to give the team the confidence to merge your stuff. When I see people constantly pushing breaking code into a CI pipeline I see an incredible amount of wasted time and shared computing resource. Especially if it's some trivial formatting check that you should have already done locally. You do what works for you, but tools like pre-commit were invented to save time and effort and they work well. > I've set up my editor correctly to auto-format and lint everything at I develop, meaning 99.9999% of the time sure the code I commit will pass all these linter checks. Then what is the problem? Have your team installed hooks that take a long time to run? Even on larger codebases pre-commit adds a negligible amount of time to each commit, unless perhaps you've touched every file in the codebase or something. Honestly your gripe with pre-commit seems mostly irrational.

wyuenho 4y ago
> The CI/CD pipeline is there to give the team the confidence to merge your stuff. That's why linting is a part of every CI stage. Linters check your code for bugs. > Have your team installed hooks that take a long time to run? Yes, they are called tests.
- globular-toast 4y ago
  Wait, you're not supposed to run tests with pre-commit. I can see why that would be frustrating. If that's the case, your team is doing it wrong.
  - wyuenho 4y ago
    Perhaps, but it's not just one team, and if you are doing code quality checks on CI anyway, I don't see the point of pre-commit (for most people).

Too 4y ago

Does it matter if it’s run twice? Usually it’s so fast you hardly even notice it. If your pre-commit takes more than 3 seconds to run it’s set up incorrectly IMO, and should belong to a more manually (and CI of course) invoked test suite instead.