How does it figure out which commit to add each change to?
Git-absorb: Git commit –fixup, but automatic
91–100 of 278 posts
Re: Git-absorb: Git commit –fixup, but automatic
#92How does it figure out which commit to add each change to?
7 lines into the README buddy:
> The command essentially looks at the lines that were modified, finds a changeset modifying those lines, and amends that changeset to include your uncommitted changes.
Re: Git-absorb: Git commit –fixup, but automatic
#93Earlier quoted context omitted.
Every team is free to choose what works best for them, but IMO always squashing PRs is not a good strategy. Sometimes you do want to preserve the change history, particularly if the PR does more than a single atomic change, which in practice is very common. There shouldn't be a static merge type preference at all, and this should be chosen on a case-by-case basis. At the risk of sounding judgemental, I think this pre…
> At the risk of sounding judgemental, I think this preference for always squashing PRs comes from a place of either not understanding atomic commits, not caring about the benefits of them, or just choosing to be lazy. In any case, the loss of history inevitably comes at a cost of making reverting and cherry-picking changes more difficult later, as well as losing the context of why a change was made. 1) Why are you e…
> The PR is the thing that gets signed-off on, and the thing that goes through the CI build/tests, so why wouldn't that be the thing kept as an atomic unit?
Because it often isn't. I don't know about your experience, but in all the teams I've worked in throughout my career the discipline to keep PRs atomic is almost never maintained, and sometimes just doesn't make sense. Sometimes you start working on a change, but spot an issue that is either too trivial to go through the PR/review process, or closely related to the work you started but worthy of a separate commit. Other times large PRs are unavoidable, especially for refactorings, where you want to propose a larger change but the history of the progress is valuable.
I find conventional commits helpful when deciding what makes an atomic change. By forcing a commit to be of a single type (feature, fix, refactor, etc.) it's easier to determine what belongs together and what not. But a PR can contain different commit types with related changes, and squashing them all when merging doesn't make the PR itself atomic.
> I don't think I've ever cared about the context for a specific commit within a PR once the PR has been merged. What kind of information do you expect to get out of it?
Oh, plenty. For one, when looking at `git blame` to determine why a change was made, I hope to find this information in the commit message. This is what commit messages are for anyway. If all commits have this information, following the history of a set of changes becomes much easier. This is helpful not just during code reviews, but after the merge as well, for any new members of the team trying to understand the codebase, or even the author themself in the future.
Re: Git-absorb: Git commit –fixup, but automatic
#94Earlier quoted context omitted.
Why store git history at all? It's useless if you don't take care of it. Have you ever used git history for anything? People use it to find the source of regressions (you can do it quite quickly using git bisect).
the git history of the linux kernel documents the Linux kernel detailed Design, the reasoning how things fit together. I've used it several times to understand why things are done the way they are. The discussion leading to this design is encoded in the revisions of the patch series, browsable on patchworks. I agree to your point that the git history has to be taken care of to be useful.
My question was, of course, rhetorical. This and hunting regressions are two great reasons to keep history.
"What did John do on Tuesday morning" is not a good reason to keep history. Neither is "This change was made as part of a sprawling "fix" commit that happened on Thursday afternoon".
The important point is if you are going to keep history at all it needs to be kept in a state that is actually useful, meaning useful for the first two things, not the last two. Otherwise it's just a waste of disk space. The git history is meticulously maintained which is what makes it actually useful.
Re: Git-absorb: Git commit –fixup, but automatic
#95How does it figure out which commit to add each change to?
it explains it in literally the first paragraph: https://github.com/tummychow/git-absorb?tab=readme-ov-file#g...
> The command essentially looks at the lines that were modified, finds a changeset modifying those lines, and amends that changeset to include your uncommitted changes.
Re: Git-absorb: Git commit –fixup, but automatic
#96Just use magit and easily make fixup! commits with like 3 key presses. Even if you don't use emacs keeping it around just to use magit is worth it. Edamagit for vscode users is not as good but it does this particular workflow great.
magit is fantastic but this isn't at all the same thing.
fixup is "stage changes, users selects past commit to fixup, commit staged changes"
absorb is "stage changes, git-absorb figures out which past commit to fixup, commit staged changes"
Re: Git-absorb: Git commit –fixup, but automatic
#97you don't want to shove them all into an opaque commit that says fixes, because you believe in atomic commits. Sure I do. The whole branch will be squashed anyway before it's merged in, and a single "fixes" commit while still on its own branch will be easier to track in a PR for addressing everything pointed out earlier. I mean, don't let me stop anyone from using this or --fixup if this is your flow, but this solves…
I wish people would stop saying "atomic commits" and start saying "main/master is stable", because that's what they actually mean. Every git commit is atomic, by definition... But people want every single possible revision to be green and buildable, which is different, and has nothing to do with git. I don't think it makes sense (tags are a lot more helpful for marking what is and isn't stable), but hey.
Yes, atomic commits is not a very descriptive term. But main/master is stable is only a necessary condition, not a sufficient one.
If you squash (or mix from the beginning) several unrelated changes into a single commit, main/master would be stable.
AFAIK atomic commits means nothing can be taken away without breaking the change and nothing needs to be added to make the change work. How to express that without 2 clauses is indeed a good question.
Re: Git-absorb: Git commit –fixup, but automatic
#98Earlier quoted context omitted.
I wish people would stop saying "atomic commits" and start saying "main/master is stable", because that's what they actually mean. Every git commit is atomic, by definition... But people want every single possible revision to be green and buildable, which is different, and has nothing to do with git. I don't think it makes sense (tags are a lot more helpful for marking what is and isn't stable), but hey.
Yes, atomic commits is not a very descriptive term. But main/master is stable is only a necessary condition, not a sufficient one. If you squash (or mix from the beginning) several unrelated changes into a single commit, main/master would be stable. AFAIK atomic commits means nothing can be taken away without breaking the change and nothing needs to be added to make the change work. How to express that without 2 clau…
Atomic commits ensure each change works independently.
Re: Git-absorb: Git commit –fixup, but automatic
#99Am I the only one who doesn't like atomic commits (or stacked PRs like graphite)? When I work on large PRs I often rewrite and move things around so much that trying to keep all commits in sync is a nightmare. I do try to split the work if it is very clearly isolated, but that usually means less than 3 PRs. I have tried graphite `gt absorb` (which might use this project?) and it still creates a mess. What I do that I…
I think this really boils down how your team is using Git and which code review tool you're using. (I've never used Gerrit personally, but as far as I understand it, we wouldn't have this conversation, since it aims to refine a single change by re-submitting a commit over and over again?) For GitHub/GitLab reviews, I'm totally with you - this makes it more convenient for the reviewer to check that/how you've responde…
Why are people talking about stacked PRs/MRs? Shouldn't they be called queued? A stack is LIFO and a queue is FIFO.
(Of course in some special case you might want to merge a later one earlier, but I don't think that's the normal case people are talking about.)
Re: Git-absorb: Git commit –fixup, but automatic
#100Earlier quoted context omitted.
Commits should be a contained change that can be understood as a logical piece of history, and reverted if necessary. If you have to look at multiple commits for 1 logical change to the code, it's much more difficult to figured out what the intention was, if it was correct, and how it can be reverted.
Do you always make all of your logical code changes in single, atomic commits? What if you have to modify a feature later on? I'm sorry, but this is ridiculous. There's nothing wrong with having multiple commits in a row modifying something. That's how git works. GitHub's PR merge workflow really messed up the meta game, I tell you what...
Yes, always. Of course I make mistakes, but the idea is that any patch I put up for review, if it cleanly applies, should not break any user and any tests. It's much more liberating knowing that bisects actually work and each commit does exactly what it says on the tin (no followup "fix"/"minor" commits that actually do something completely different). Incremental improvements are compatible with this approach. Also see https://gist.github.com/thoughtpolice/9c45287550a56b2047c631... .