Web lists-archives.com

Re: Commit dropped when swapping commits with rebase -i -p




Hi,

Sebastian Schuberth wrote:
> On Wed, Aug 30, 2017 at 8:07 PM, Martin Ågren <martin.agren@xxxxxxxxx> wrote:

>> The man-page for git rebase says that combining -p with -i is "generally
>> not a good idea unless you know what you are doing (see BUGS below)".
>
> Thanks for pointing this out again. I remember to have read this some
> time ago, but as I general consider myself to know what I'm doing, I
> forgot about it :-)

Heh.

> Anyway, this should really more explicitly say *what* you need to know
> about, that is, reordering commits does not work.

It tries to explain that, even with an example.  If you have ideas for
improving the wording, that would be welcome.

That said, ...

>> So if you agree that a "dropped commit" is a "counterintuitive result",
>> this is known and documented. Maybe the warning could be harsher, but it
>> does say "unless you know what you are doing".
>
> I'd say it's worse than counterintuitive, as counterintuitive might
> still be correct, while in my case it clearly is not. So yes, the
> warning must be harsher in my opinion. Maybe we should even abort
> rebase -i-p if reordering of commits is detected.

This sounds like a more promising approach.  If you can detect when
the rebase -i -p is going to cause trouble, then I would be all for
aborting.  If you want to be extra nice to people, you can provide a
--force escape valve to let them experience the broken behavior, but I
don't think that is necessary.

I also think a loud warning when -i -p is used even when it is not
going to cause trouble would be a valuable change.  E.g. maybe the
template that opens in the editor could say something about reordering
commits not being advisable?

E.g. I could imagine the todo list including some instructions in the
spirit of

	# git rebase --preserve-merges does not support reordering commits.
	# To attempt reordering anyway, add a line with the text "reorder".
	# It is not likely to behave as you expect.  You have been
	# warned.

Thanks,
Jonathan