1

Let's say I have a branch Master With the following commits:

M1->M2

Then from M2 a branch Feature is created and developed as:

M2->F1->F2->F3->F4

Then a merge request is created from Feature and a code review is done where an error is found in F1 (let's say a typo).

The options I don't like are:

  1. Creating a new commit fixing the typo => when merging both the mistake and the correction will be kept, instead of only keeping "the good version" of F1.

  2. git commit --amend and fix => this will again keep the error and the correction in history, just that it will be corrected in F4.

  3. Doing an interactive git rebase => This will actually look good in the history, but it might be annoying as it's likely to happen that there will be Conflicts with all F2, F3 and F4 commits.

So my question is:

Is there an appropriate way of fixing early errors in not-yet-merged branches?

Ivan
  • 1,352
  • 2
  • 13
  • 31
  • 2
    The "right" way to handle this is a matter of opinion. For StackOverflow purposes, there are only technically-correct ways (that produce the desired files) and technically-incorrect ways (that don't); your three options that you don't like are all technically correct. Since this is a comment, I'll mention that my personal preference (my opinion) is that option 3 *tends* to be best as long as the feature branch is not generally available / in-use yet. Otherwise option 1 tends to be best. – torek May 19 '22 at 18:13
  • 2
    I agree with @torek. I may lean even stronger. In a PR/MR workflow, personal feature branches are rarely ever sacred until they are merged. I would choose option 3 every time. In my experience (many thousands of rebases?), cascading conflicts are rare. Though, that could be because I generally keep most of my modifications to each file in a single commit, when possible. For example, if I modify the same file 5 commits in a row, I'll likely squash those into one with a detailed single commit message. You could also try the rebase, and if you get cascading conflicts, abort, squash, then retry. – TTT May 19 '22 at 20:19
  • I should clarify, my previous comment refers to a "typo" or minor bug fix as you mentioned. If the fix is a refactor, conflicts would be far more likely, and then I'd either squash or just add a new commit, if I still wanted my previous version to be in the history. – TTT May 19 '22 at 20:24

1 Answers1

0

The appropriate way of fixing the error is exactly the one that you don't like. When you push branch to the remote you should not change history of that branch. New commit is the only way to fix the change. Please also notice, that fixing error may be helpfull for the future reference to other developers when they see your fix.

The case may be different if you never pushed branch, and you found the typo or whatever problem and want to fix. Then you can proceed with interactive rebase. Please refer to this question

But one note. You should ask different question. How to avoid typo and other problems in the commit, in the first place? And there are a lot of ways to do it. One of them would be using git pre-commit hooks.

kadewu
  • 336
  • 4
  • 8
  • I want to add some comment, You can actually try to make approach like this: Introduce new branch apply commits from previous branch and fix the commits in the process. But I do not want to add it to the answer, as this is not the approach I would state as correct. – kadewu May 19 '22 at 10:29
  • 1
    In Git, absolutes are frowned upon. (So never use them? ) – TTT May 19 '22 at 20:58
  • I'm not sure, to what you are referring now? – kadewu May 19 '22 at 23:15
  • By absolutes, I mean, "Never do X." or "Always do X". Your bolded statement is an absolute because you're saying you should never force push. I think that's far too strong of a statement, since of course there are valid reasons to force push. And then I jokingly said to never use absolutes! (Which is itself an absolute.) :D – TTT May 20 '22 at 02:15
  • Ohh you mean that. I was honestly wondering 'why suddenly relating to absolutes paths' :D Yeah. I fully agree with your statement. I even myself actually breaks that statement (as i love rebase over merge). But one thing is that I didn't use 'never', second I believe is good to tell new users, to not do that. This is just cause I've seen in my work people that didn't understand what they were doing. – kadewu May 20 '22 at 11:26