Stop running hooks in commit function

[mtsgrd]

• will be orchestrated by client instead

---

This is part 2 of 3 in a stack made with GitButler:

•  3  Refactor git commit hooks #5940
•  2  Stop running hooks in commit function #5894 👈
•  1  Make it easier to pass diffs to list vbranch function #5890

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
gitbutler-components	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 14, 2025 8:39am
gitbutler-web	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 14, 2025 8:39am

[krlvi]

I think you should make commit no longer run hooks here at all. Instead, having the hooks execution in a separate command, this way the UI can execute the two commands in sequence.
As I wrote in the other message before, the benefits would be:

• better UX (we can show that hooks are being executed/spinner)
• we can easily differentiate the errors
• the test is more testable and less error prone because each command does less.
  Let's discuss tomorrow.

[Byron]

I love the idea to make the UI more aware of what's going on. Something I wonder though is if there is another way of doing it, one that doesn't force the UI to steer it. An idea was to stream progress information to the UI, but I don't think tauri supports that outside of events, so for the UI it would always be a separate stream of events coming in so the action at hand and additional streaming data (like progress) will always be separate. In the backend, this could probably be some sort of trait implementation that is passed around. With such a system, progress could be implemented, too.

Both ideas, separating commits from running hooks, and passing some trait object that allows for real-time metadata to get out of backend functions while they execute, are orthogonal and maybe both will help solve this problem better than they would alone.

I haven't arrived at a clear conclusion myself, but wanted to share my thoughts while at it.

[krlvi]

I love the idea to make the UI more aware of what's going on. Something I wonder though is if there is another way of doing it, one that doesn't force the UI to steer it. An idea was to stream progress information to the UI, but I don't think tauri supports that outside of events, so for the UI it would always be a separate stream of events coming in so the action at hand and additional streaming data (like progress) will always be separate. In the backend, this could probably be some sort of trait implementation that is passed around. With such a system, progress could be implemented, too.

Both ideas, separating commits from running hooks, and passing some trait object that allows for real-time metadata to get out of backend functions while they execute, are orthogonal and maybe both will help solve this problem better than they would alone.

I haven't arrived at a clear conclusion myself, but wanted to share my thoughts while at it.

Another alternative could be to compose the two commands, this way the UI doesn't have to do it. But it feels like this may not be the correct separation since we want the UI to be aware of the two steps in this process

[krlvi]

👨‍🔬 Meta comment on stacking / code collab /review 🔍

Interestingly, this time stacking makes the whole thing harder to review (e.g. the changes feel very much related with new code being introduced and then moved to a different file in another part of the stack).
Also our chat/messages become scattered between the stacked branches.

As a reviewer, I think would have been more ergonomic to have these as commits in one branch. And then I would likely look at the final diff instead of commit by commit...