Re: [PATCH 5/8] rebase: introduce the --recreate-merges option

[Sergey Organov <sorganov@gmail.com>]

Hi Johannes,

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi Sergey,
>
> On Fri, 9 Feb 2018, Sergey Organov wrote:
>
>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>> 
>> [...]
>> 
>> > With this patch, the goodness of the Git garden shears comes to `git
>> > rebase -i` itself. Passing the `--recreate-merges` option will generate
>> > a todo list that can be understood readily, and where it is obvious
>> > how to reorder commits. New branches can be introduced by inserting
>> > `label` commands and calling `merge - <label> <oneline>`. And once this
>> > mode has become stable and universally accepted, we can deprecate the
>> > design mistake that was `--preserve-merges`.
>> 
>> This doesn't explain why you introduced this new --recreate-merges. Why
>> didn't you rather fix --preserve-merges to generate and use new todo
>> list format?
>
> Because that would of course break existing users of
> --preserve-merges.

How exactly? Doesn't "--recreate-merges" produce the same result as
"--preserve-merges" if run non-interactively?

> So why not --preserve-merges=v2? Because that would force me to maintain
> --preserve-merges forever. And I don't want to.
>
>> It doesn't seem likely that todo list created by one Git version is to
>> be ever used by another, right?
>
> No. But by scripts based on `git rebase -p`.
>
>> Is there some hidden reason here? Some tools outside of Git that use old
>> todo list format, maybe?
>
> Exactly.
>
> I did mention such a tool: the Git garden shears:
>
> 	https://github.com/git-for-windows/build-extra/blob/master/shears.sh
>
> Have a look at it. It will inform the discussion.

I've searched for "-p" in the script, but didn't find positives for
either "-p" or "--preserve-merges". How it would break if it doesn't use
them? What am I missing?

>
>> Then, if new option indeed required, please look at the resulting manual:
>> 
>> --recreate-merges::
>> 	Recreate merge commits instead of flattening the history by replaying
>> 	merges. Merge conflict resolutions or manual amendments to merge
>> 	commits are not preserved.
>> 
>> -p::
>> --preserve-merges::
>> 	Recreate merge commits instead of flattening the history by replaying
>> 	commits a merge commit introduces. Merge conflict resolutions or manual
>> 	amendments to merge commits are not preserved.
>
> As I stated in the cover letter, there are more patches lined up after
> this patch series.

Good, but I thought this one should better be self-consistent anyway.
What if those that come later aren't included?

>
> Have a look at https://github.com/git/git/pull/447, especially the latest
> commit in there which is an early version of the deprecation I intend to
> bring about.

You shouldn't want a deprecation at all should you have re-used
--preserve-merges in the first place, and I still don't see why you
haven't. 

>
> Also, please refrain from saying things like... "Don't you think ..."
>
> If you don't like the wording, I wold much more appreciate it if a better
> alternative was suggested.

Sorry, but how can I suggest one if I don't understand what you are
doing here in the first place? That's why I ask you.

>
>> Don't you think more explanations are needed there in the manual on
>> why do we have 2 separate options with almost the same yet subtly
>> different description? Is this subtle difference even important? How?
>> 
>> I also have trouble making sense of "Recreate merge commits instead of
>> flattening the history by replaying merges." Is it "<Recreate merge
>> commits by replaying merges> instead of <flattening the history>" or is it
>> rather "<Recreate merge commits> instead of <flattening the history by
>> replaying merges>?
>
> The documentation of the --recreate-merges option is not meant to explain
> the difference to --preserve-merges. It is meant to explain the difference
> to regular `git rebase -i`, which flattens the commit history into a
> single branch without merge commits (in fact, all merge commits are simply
> ignored).

Yeah, that's obvious, but the point is that resulting manual is ended
up being confusing.

> And I would rather not start to describe the difference between
> --recreate-merges and --preserve-merges because I want to deprecate the
> latter, and describing the difference as I get the sense is your wish
> would simply mean more work because it would have to be added and then
> removed again.

I suspect you actually didn't need those new option in the first place,
and that's the core reason of these troubles.

-- Sergey