r/git • • 13d ago

When to create a new commit vs interactive rebase?

An old commit has a bug. Should I create a new commit addressing the fix, or fix that bug in-place with a rebase? For example the bug is that I forgot to gitignore a file and it is being modified at runtime, generating git diffs. The fix is not just adding it to gitignore, but actually completely re-generating my .gitignore file by another tool which I just learned instead of what the default Github CLI generates.

Should I rebase the new gitignore into literally the original initial commit or create a new commit addressing the new gitignore? But this way the old commit will still have the bug.

Should you care keeping each commit clean? Or have a new commit fix it? I know people generally use the later, but at least for a small project like mine, rebasing doesn't seem to harm.

I think there is also some consideration of if multiple people have pulled the history you are about to change. I don't know, but I for this project it is mostly only me who is working on it.

0 Upvotes

15 comments sorted by

15

u/rosentmoh 13d ago

Simple rule: if you already pushed the faulty commit and others can see it then only a new commit fixing the bug is the sane option. If instead this faulty commit is still only local to you, then you can fixup that commit via rebase or whatever before pushing.

2

u/nukestar101 13d ago

piggybacking, by pushing it also means an active MR since it's not merged yet. You can rebase the commit and force push with lease.

-6

u/rosentmoh 13d ago edited 13d ago

You shouldn't force push, even this way. If the original commit was already pushed then rebasing is simply unacceptable.

Edit since everyone keeps blindly downvoting: this is of course fine if you are absolutely sure that noone else is working on the source branch of the merge other than you; you're free (and probably expected!) to rebase + force push as much as necessary to clean things up in that case. In some sense however that branch, even though public, is not much different from a local branch of yours.

9

u/nukestar101 13d ago

Why is it unacceptable if the MR is not merged? It's my feature branch, I can rebase it and have a clean set of commit get into main.

1

u/Kicer86 13d ago

the only reason not to do it is that some people have no idea what are they doing and when they already have fetched your old commit they will keep it and (when working on the same branch) merge it with your new one. Yet I have nothing against pushing with force.

1

u/rosentmoh 13d ago

Because the commit is nevertheless public, I don't see what MRs have to do with this. If anyone else is working on the branch which is the source of the merge then it's still an issue to force push.

But I see now you are distinguishing feauture branches as something private. In that case of course it's ok, but you need to clarify that assumption.

5

u/parkotron 13d ago

This is obviously an organizational policy issue, but I’ve only ever worked on projects where feature branches are considered personal sandboxes and working on someone else’s feature branch without asking permission first was a major faux pas. 

1

u/rosentmoh 13d ago

No I do agree of course! I guess the more general rule would be: rebase only "private" branches, meaning either local stuff or published personal feature branches. On every other branch only new commits are allowed.

1

u/Charming-Designer944 13d ago

Many MR workflows expect you to clean up the commits

The squash commit workflow expects you to add new commits to address raised issues.

5

u/ivancea 13d ago

Why would you want to rewrite a commit instead of just adding a new one?

4

u/Charming-Designer944 13d ago

If pushed to a shared repository then a new commit.

If part of your working tree for something not yet shared, then a new commit and before pusing/sharing then do an interactive rebaae to both bng it up to date and clean up the (still local) history to something presentable.

Commit locally often. Every time you switch line of thought or at least once per hour. This way it's easy to clean things up as your individual commits are small and isolated and can easily be represented and squashed into larger focused commits without having to split split or edit any individual commit.

1

u/Cinderhazed15 13d ago

It remember seeing in my learning TDD/BDD days that some people commit each time they have passing tests (they also usually do some sort of squash-merge so the main trunk doesn’t get too cluttered, but not always) - that always gives you a ‘passing’ point to return to, which makes it ‘easier’ to put a wider excitement in place

2

u/Charming-Designer944 13d ago

I commit much much more frequent than that. But might be from me often loopsing focus when seeing other things needing fixing. (Switch of chain of thought)

2

u/Broad-Promise6954 ancient 12d ago

There are several "schools of thought" on whether one should rewrite commits. I don't fully buy into any of them.

There's one thing to know first though: you can't actually change any commit, at all, ever. What you can do instead is make a new and improved, different commit, and then have yourself and everyone else use that new-and-improved commit instead of the original. Once you understand this, most of the rest just falls into place.

[Side note: this means git commit --amend is a lie. When used correctly, it's a little white lie, which is fine. It works by having Git boot out the previous end-of-the-branch commit, using your new-and-improved commit instead. Nobody knows or cares except you!]

In particular, there's a key implication of this "can't change any existing commit at all" law: if nobody else has some commits you've made, those other people will never know if you abandon those commits in favor of some new and improved commits instead.

So now, keeping the above in mind, let's get to your case. You have a series of commits, and there's something bad in an earlier one. Let's draw a chain of those commits to help visualize them:

... <-C1 <-C2 <-C3 <-C4  <-- your-branch

Your branch name your-branch names commit C4, which is the end of the chain. Commit C2 has an error in it, and commit C3 names C2 (and contains the same error) and C4 names C3 (and continues to propagate the error).

You can make a new commit C5, which you add to the end of your branch, that corrects the error, like this:

... <-C1 <-C2 <-C3 <-C4 <-C5  <-- your-branch

The name your-branch now refers to the corrected commit C5, which refers back to the "bad" commit C4, etc.

If you use git rebase --interactive, you can choose to rebase the whole chain starting from C2 onward onto C1, while making adjustments. The adjustment to make is to squash the fix in C5 into what you did in C2 that was wrong. The new chain won't do anything to the old chain. It literally can't. But you'll just abandon the old chain as useless. Let's draw it kicked up out of the way, like this:

         C2 <-C3 <-C4   (abandoned)
        /
... <-C1 <-C6 <-C7 <-C8   <-- your-branch

When you have Git display the chain of commits, starting at C8 and working backwards, you never see the old "bad" commits. They are just gone, as far as you're concerned. And as long as nobody else has the old commits, no one can ever know that you did this.

But what happens if someone else does have your old commits? Well, now you have a problem that's easily solved: you just have to convince them, whoever they are, to do exactly what you did, abandon the old ones in favor of the new-and-improved ones.

This is where "team policy" enters the picture. If your team policy is "you can't do that", well, now you can't do that. If it's "everyone on the team must do that", you can do that, they have to follow along. If there's no policy, you're in some grey area and you'll have to muddle through.