Re: Doubt about skipped commits

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Feb 09 2023, Vinayak Dev wrote:

> Hello everyone!
> I am a new git contributor, and was reading through the source code.
> I want to implement a small check that prevents an interactive rebase
> from proceeding with a fixup in case the last commit did not apply
> cleanly, in which case the fixup could amend the wrong commit.

Hi, and welcome!

> I am doubtful about how to check for missing/skipped commits in
> rebase-interactive.c .
> Could anyone provide guidance?

I think it's probably best for this sort of thing to start with a test
case you think is doing the wrong thing, i.e. can you skim the
t/*rebase*.sh tests we have, and see if you could amend one to produce a
test case that fails now, but which you expect to pass if you were to
implement this.

I'm asking because if you mean that you have e.g.:

	pick B
	pick A
	fixup D
	pick C

And we apply B, but then have a conflict with A, and drop you into the
shell, you need to fix that etc. (maybe extensively), and you then want
"D" to categorically fail to apply...

...then that's something that a lot of people definitely rely on working
the way it does now.

In general just because your commit failed to apply cleanly it doesn't
have anything per-se to do with whether you want to fix up a later
commit into that failed-to-apply commit.

E.g. consider a case where "D" is a typo fix for some documentation
presnet in "A", but the reason we failed to apply "A" is because of a
conflict in code that A and B both modify, but which is far removed from
the doc change.

And even *if* it's all to the same few lines of code it's not obvious
that we don't want the fix-up still, we just want to resolve the
conflict and move on.

E.g. I might add a param to a function, and typo the parm name, and "D"
is the fix-up for that typo, but "B" added another parameter, I still
want to churn ahead and apply this all in that order, conflicts and all.

Which is not to say that I don't think this *could* be useful,
e.g. could we be smart enough to say "hey, this would apply, but not
anymore because this other thing got re-arranged"?

Or maybe you're talking about something else entirely.

But in any case, coming up with some testcase or demo for what you think
is the wrong thing might save you some grief, in case someone annoying
like myself comes along and says "actually that's intentional, and a lot
of people rely on it" :)