4 ms·

I wonder if the complexity of fixing trivial code mistakes in CI is worth it compared to catching them in a pre-commit hook.

yeswecatan 2y ago
Unfortunately people will use --no-verify to bypass hooks.
- normie3000 2y ago
  I don't understand commit hooks - they're like binding a macro to the MS Word save button to make it conditional.
  - chuckadams 2y ago
    > like binding a macro to the MS Word save button to make it conditional You have no idea how much I'd love that feature. Inasmuch as "save" is still a thing anyway. I don't miss explicit saves in IDEA, I see commit as the "real" save operation now, and I don't mind being able to hook that in an IDE-independent way. I think the UX of git hooks has been sub-par for sure, but tools like the confusingly named pre-commit are helping there.
  - sgarland 2y ago
    Because if you haven’t auto-formatted, lined, etc. then it’s a very easy way to do that so you don’t waste time watching CI fail for something stupid like trailing comma placement. I don’t want to think about formatting, I just want everything to be consistent. A pre commit hook can run those tools for me, and if any changes occurred, it can add them to the commit.
- PhilipRoman 2y ago
  You can put hooks on the server side of git. It can do pretty much anything that CI/CD can.
  - yeswecatan 2y ago
    That requires Github Enterprise (if using GH, of course), no?
    - PhilipRoman 2y ago
      Well it requires a server and 5 minutes of your time :) I guess you can always have it as a mirror for your GH repository. Gitlab has push mirroring, not sure about GH: https://docs.gitlab.com/user/project/repository/mirror/push/ https://docs.gitlab.com/user/project/repository/mirror/push/
- hinkley 2y ago
  There's a long set of steps to making a tool mandatory in a development environment, but the final step should always, always be, "And you will find yourself on a PIP if you refuse to use the mandatory tools." If people want to die on a hill that is demonstrably causing problems for all of their coworkers then let em.
  - yeswecatan 2y ago
    Oh how I wish engineering leadership would actually mandate certain things such as this.
    - hinkley 2y ago
      They always pick the wrong things to mandate don't they.
- anttiharju 2y ago
  Enforce on CI. Autofix in pre-commit hooks. Lefthook is fantastic for this. Example config: https://github.com/anttiharju/vmatch/blob/9e64b0636601c236a55cd2152cb82d7cd19f5451/lefthook.yml https://github.com/anttiharju/vmatch/blob/9e64b0636601c236a5...
- michpoch 2y ago
  Then they'll lose time for the same verifications to fail in the PR?
matharmin 2y ago
In my opinion neither hooks nor CI should ever make changes to code automatically. When I commit changes, I want to see exactly what I commit, and not have some system change it at the last minute. Instead, have tooling to do that before committing (vscode format-on-save, or manually run a task), then have a pre-commit hook just do a sanity-check on that. It only needs to check modified files, so usually very fast. Then, have an additional check on CI to verify formatting on all files. That should rarely be triggered, but helps to catch cases where the hooks were not run, for example from external contributes. That also makes it completely fine for this CI step to take a couple of minutes - you don't need that feedback immediately.
hinkley 2y ago
The tension in any system is how many ways the build can fail other than the most obvious one. So I generally only encourage things in the pre-commit hook like no weird punctuation (I'm looking at you, Microsoft), no empty commit messages, and maybe require a ticket number (or try to guess one out of the branch name). Though it would be sort of interesting or maybe just amusing if you made something like ssh-agent but for 'git commit' and your test runner. Only allow commits when all files are older than your last green test run.

yeswecatan 2y ago

Unfortunately people will use --no-verify to bypass hooks.

normie3000 2y ago
I don't understand commit hooks - they're like binding a macro to the MS Word save button to make it conditional.
- chuckadams 2y ago
  > like binding a macro to the MS Word save button to make it conditional You have no idea how much I'd love that feature. Inasmuch as "save" is still a thing anyway. I don't miss explicit saves in IDEA, I see commit as the "real" save operation now, and I don't mind being able to hook that in an IDE-independent way. I think the UX of git hooks has been sub-par for sure, but tools like the confusingly named pre-commit are helping there.
- sgarland 2y ago
  Because if you haven’t auto-formatted, lined, etc. then it’s a very easy way to do that so you don’t waste time watching CI fail for something stupid like trailing comma placement. I don’t want to think about formatting, I just want everything to be consistent. A pre commit hook can run those tools for me, and if any changes occurred, it can add them to the commit.
PhilipRoman 2y ago
You can put hooks on the server side of git. It can do pretty much anything that CI/CD can.
- yeswecatan 2y ago
  That requires Github Enterprise (if using GH, of course), no?
  - PhilipRoman 2y ago
    Well it requires a server and 5 minutes of your time :) I guess you can always have it as a mirror for your GH repository. Gitlab has push mirroring, not sure about GH: https://docs.gitlab.com/user/project/repository/mirror/push/ https://docs.gitlab.com/user/project/repository/mirror/push/
hinkley 2y ago
There's a long set of steps to making a tool mandatory in a development environment, but the final step should always, always be, "And you will find yourself on a PIP if you refuse to use the mandatory tools." If people want to die on a hill that is demonstrably causing problems for all of their coworkers then let em.
- yeswecatan 2y ago
  Oh how I wish engineering leadership would actually mandate certain things such as this.
  - hinkley 2y ago
    They always pick the wrong things to mandate don't they.
anttiharju 2y ago
Enforce on CI. Autofix in pre-commit hooks. Lefthook is fantastic for this. Example config: https://github.com/anttiharju/vmatch/blob/9e64b0636601c236a55cd2152cb82d7cd19f5451/lefthook.yml https://github.com/anttiharju/vmatch/blob/9e64b0636601c236a5...
michpoch 2y ago
Then they'll lose time for the same verifications to fail in the PR?

matharmin 2y ago

In my opinion neither hooks nor CI should ever make changes to code automatically. When I commit changes, I want to see exactly what I commit, and not have some system change it at the last minute. Instead, have tooling to do that before committing (vscode format-on-save, or manually run a task), then have a pre-commit hook just do a sanity-check on that. It only needs to check modified files, so usually very fast. Then, have an additional check on CI to verify formatting on all files. That should rarely be triggered, but helps to catch cases where the hooks were not run, for example from external contributes. That also makes it completely fine for this CI step to take a couple of minutes - you don't need that feedback immediately.

hinkley 2y ago

The tension in any system is how many ways the build can fail other than the most obvious one. So I generally only encourage things in the pre-commit hook like no weird punctuation (I'm looking at you, Microsoft), no empty commit messages, and maybe require a ticket number (or try to guess one out of the branch name). Though it would be sort of interesting or maybe just amusing if you made something like ssh-agent but for 'git commit' and your test runner. Only allow commits when all files are older than your last green test run.