Hi Jacob,
Jacob Keller <jacob.keller@xxxxxxxxx> writes:
> On Wed, Apr 11, 2018 at 10:42 PM, Sergey Organov <sorganov@xxxxxxxxx> wrote:
>> Hi Jacob,
>>
>> Jacob Keller <jacob.keller@xxxxxxxxx> writes:
>>> On Wed, Apr 11, 2018 at 6:13 AM, Sergey Organov <sorganov@xxxxxxxxx> wrote:
>>>> It was rather --recreate-merges just a few weeks ago, and I've seen
>>>> nobody actually commented either in favor or against the
>>>> --rebase-merges.
>>>>
>>>> git rebase --rebase-merges
>>>>
>>>
>>> I'm going to jump in here and say that *I* prefer --rebase-merges, as
>>> it clearly mentions merge commits (which is the thing that changes).
>>
>> OK, thanks, it's fair and the first argument in favor of --rebase-merges
>> I see.
>>
>
> I'd be ok with "--keep-merges" also. I don't like the idea of
> "flatten" as it, to me, means that anyone who wants to understand the
> option without prior knowledge must immediately read the man page or
> they will be confused. Something like "--rebase-merges" at least my
> coworkers got it instantly. The same could be said for "--keep-merges"
> too, but so far no one I asked said the immediately understood
> "--no-flatten".
If they got --rebase-merges instantly, they should already have known
what "rebase" and "merge" mean. If so, they are likely Git users that
are already familiar with "git rebase" and thus at least heard about a
buddy called --preserve-merges. If it's the case indeed, the outcome
you've got was rather predictable, me thinks.
Now, what are the consequences?
When pleasing maximum number of users of --preserve-merges (and probably
--recreate-merges) is number one target of design, while the rest of
issues are secondary, being in favor of --rebase-merges, --keep-merges,
or --<whatever>-merges is only natural indeed.
However, I don't believe meeting user expectations should be the number
one criteria of a good design. Sound technical design should come first,
and meeting user expectations, provided they don't contradict the
design, only second. That's how Git was born, that's how it should
continue to evolve. Going in reverse direction, from user expectations
to design, will give us Bzr, not Git.
In discussing of these patch series though I rather see care for user
expectations or preferences being used as an excuse for questionable
design all the time. That's what actually bothers me much more than
choosing particular names for particular options.
Narrowing back to the topic, don't you see, honestly, that there is
something wrong with:
git rebase --rebase-merges
that is supposedly easy to understand even without referring to the
manual, yet when you do happen to refer to the manual, you suddenly
realize it's not that easy to understand:
--rebase-merges[=(rebase-cousins|no-rebase-cousins)]
Rebase merge commits instead of flattening the history by replaying
merges. Merge conflict resolutions or manual amendments to merge
commits are not rebased automatically, but have to be applied
manually.
???
Please read the description. Actually read as if you never knew what's
all this about.
Why does it use "flattening the history" that is supposedly hard to
understand to explain "--rebase-merges" that is supposedly easy to
understand? How comes? And if it's actually a good explanation, why
didn't author just call the option --no-flatten-history, to match its
description?
Next, what is "replaying merges", exactly? That's explaining one term
with another that has not being explained and sounds being even more
vague.
Further, "Merge conflict resolutions or manual amendments to merge
commits are not rebased automatically, but have to be applied manually."
is mutually exclusive with "Rebase merge commits", making all this even
more messy. A merge commit is just content with multiple parents, and
`git rebase`, by definition, reapplies the changes the content
introduces. Any "amendments" or "resolutions" that could have been
happening (or not) when that commit was being created are entirely
irrelevant.
Further yet it goes with:
"By default, or when `no-rebase-cousins` was specified, commits which do
not have `<upstream>` as direct ancestor will keep their original branch
point."
Really? What does it actually mean? What is "commit branch point",
exactly? What "direct ancestor" means in this context, exactly? Provided
even when I do know what the option actually does, the description looks
wrong, how it could explain anything?
Having all this right here in the patch series, you guys try to convince
me that it should not be fixed? That it meets user expectations? You
know what? I, as a user, have somewhat higher expectations.
Below is my final attempt at actually defining a sane alternative. If
you still find this approach inferior, please feel free to ignore it. I
added "history" at the end of original --no-flatten, as a courtesy to
user expectations, as you seem to prefer more verbose names:
----------
--flatten-history
Flatten rebased history by reapplying non-merge commits only.
This is the default.
--no-flatten-history[=<options>]
Do not flatten rebased history. When this option is specified,
the original shape of the history being rebased will be
preserved. <options> is comma-separated list of supported
options.
The following options are supported:
'merge-heads' - perform a merge of rebased merge heads instead of
rebasing original merge commits. Only commit messages will be taken from
original merge commits in this mode.
'rebase-cousins' - commits which do not have <upstream> as
direct ancestor will not keep their original branch point.
------------
In fact I think that 'rebase-cousins' should be removed as making no
sense, but I've borrowed it from the original anyway, to show how the
concept of this option itself works to support multiple additional
options.
-- Sergey
