← Back to context

Comment by Zambyte

8 months ago

> The Acked-By and mailing list discussion link examples don't seem to be good examples. Both of these are likely already known when the commit is made.

Discussion regarding a commit (is: review) and acknowledgment of a commit cannot happen before the commit has been made.

> One use case I think might be a better example is to add a git note to a commit that has later been reverted.

Commit messages are better for this use case. When you got blame a file, it shows the latest changes for that file. If a commit reverts changes from another commit, the newer commit that reverts the older commit will show up in the blame.

22 comments

Zambyte

Reply
saghm  8 months ago

> Discussion regarding a commit (is: review) and acknowledgment of a commit cannot happen before the commit has been made.

It can't happen before the commit on a feature branch, but it can happen before merging the commit back to the main development branch. Given that a rebase or merge commit is already frequently necessary to integrate changes from a feature branch after review is finished, I don't see why this type of info couldn't be added (or even required to exist) before merging.

  • Pxtl  8 months ago

    The history-destroying problems of rebasing are a rant on their own.

    • Zambyte  8 months ago

      That's a UI problem with git making it hard to find hidden commits (pre-rebase). The commits aren't destroyed, they are hidden. The Jujutsu CLI is nice because it fixes this UI problem.

      2 replies →

    • saghm  8 months ago

      I don't think I agree with this take, at least not completely. I tend to commit quite frequently when working on a feature branch in ways that wouldn't be desirable to include in the history of the main development branch, and I take advantage of the fact that rebase lets me clean everything up beforehand into whatever commits I actually want afterwards (which I usually do two separate times, once before opening a review so I can ensure that the diffs help make things easier to read, e.g. if I need to include changes from another branch or make changes in the codebase that aren't directly related to what I'm working on but still are useful for my changes for some reason), and then again after the review is complete if I needed to make additional changes that don't belong in a separate commit (because I find that reviews are cleaner when rebases don't take place during them, and a review that requires more than one or two follow-up commits generally tends to be due to issues that need to be addressed with an offline discussion that I can come back afterwards with structural changes that make the existing diff less relevant). Having to preserve the exact history of commits during my development would be a bad thing in my opinion, since it would either require including lots of small unhelpful commits into the history of the main development branch or discourage committing as often, either of which I think would be a mistake.

      Where I agree with your take partially is that the UX for all of this in git is not great, and that ends up meaning that most people don't actually use git in this way. If the process of manually structuring the commits to clean up history on a feature branch were more intuitive, then I'd predict the issues of history-destroying rebases to essentially be moot; everyone would just present the commits for review exactly as we'd want them before review, and then we'd fast-forward merge into the main development branch without any issue. The problem is that doing this sort of restructuring after the fact isn't actually easy to learn how to do because of the poor ergonomics of the git CLI, so it's both tedious and error-prone for almost everyone. My perspective is that most of the concern around messing with history in git comes from being burnt by changes that were not intended by the one making them, and that workflows that avoid it (like merge commits) are essentially a compromise to avoid that risk by accepting a certain amount of cruft in the history of the main development branch. I don't blame anyone for using them, since the problems that make the rebase workflow harder are very real, but I don't think that the fact that rebase changes history is the real issue as much as it provides a mechanism for the actual underlying issues to manifest.

      6 replies →

hinkley  8 months ago

You’re treating a commit as an atom, which is not true in patch based git situations like Linux.

Most of the rest of us do not work this way, but they still do. The rest of us also only have to deal with three way merges most of the time, instead of octopus merges. Though I jokingly call, “fixing an incorrect three way merge” a “five way merge” because you end up doing a star shaped pattern of diffs to re-resolve the code to retain the intents of all three versions. A to merge, B to merge, merge to HEAD~, A to HEAD~ and B to HEAD~