From: Junio C Hamano <gitster@pobox.com>
To: Elijah Newren <newren@gmail.com>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org,
Philippe Blain <levraiphilippeblain@gmail.com>,
Johannes Sixt <j6t@kdbg.org>,
Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH v2] range-diff: optionally include merge commits' diffs in the analysis
Date: Tue, 12 Nov 2024 09:29:03 +0900 [thread overview]
Message-ID: <xmqqwmh98fcg.fsf@gitster.g> (raw)
In-Reply-To: <CABPp-BFXZ+aaTWGAUhJxh2YYZ131BNYFCyLNVbjntBmStUu0SA@mail.gmail.com> (Elijah Newren's message of "Mon, 11 Nov 2024 08:51:12 -0800")
Elijah Newren <newren@gmail.com> writes:
>> There _may_ need a tweak of the matching algorithm to allow the
>> "same" merge on both sides to match, even if they diverge a lot,
>> though. A range-diff that pairs a merge in the previous iteration
>> with a single patch in the updated iteration may be hard to read.
>
> Sounds like you are arguing that there is an angle/usecase from which
> `first-parent` makes sense, namely the viewpoint that "a merge is
> merely bringing in lots of changes into the mainline as a single unit"
> as you put it. What was surprising to me, though, is that it's a
> completely different usecase than the one that was brought up in the
> commit message for this feature, namely "so-called 'evil merges' are
> sometimes necessary and need to be reviewed too".
What I had in mind when I wrote the example you are responding to is
based on what sometimes happens while I make repeated merges (and as
you may know, I make lots of them). In the first attempt I miss the
fact that I need semantic adjustments and then in the second attempt
I know what additional changes are necessary in the same merge (i.e.
merging exactly the same iteration of the same topic). If you run
the first-parent range-diff between one iteration of 'seen' and
another, the "additional changes" I would make in the second attempt
would be the only thing that will appear in the output, showing the
"evil merge".
There can also be updates in the topic itself when I rebuild 'seen',
in addition to merge-fixes to adjust for semantic conflicts. Such a
change would also appear in the first-parent view. If you used
other views, like dense-combined or remerge-diff, these updates in
the topic itself may be hidden, as these other views are
specifically designed to highlight conflict resolutions and evil
merges by discarding mechanically resolvable changes. So it all
depends on what you are looking for and what you are deliberately
excluding from the lower level of diff generation when you prepare
your range-diff, which is a "diff of diff". Giving an impression
that first-parent is the most useful may risk misleading readers in
that sense.
next prev parent reply other threads:[~2024-11-12 0:29 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-07 17:20 [PATCH] range-diff: optionally include merge commits' diffs in the analysis Johannes Schindelin via GitGitGadget
2024-11-08 3:46 ` Junio C Hamano
2024-11-08 6:53 ` Johannes Sixt
2024-11-08 10:53 ` Johannes Schindelin
2024-11-09 8:49 ` Elijah Newren
2024-11-11 0:20 ` Junio C Hamano
2024-11-08 11:56 ` Junio C Hamano
2024-11-08 15:53 ` Elijah Newren
2024-11-08 13:43 ` [PATCH v2] " Johannes Schindelin via GitGitGadget
2024-11-08 17:04 ` Elijah Newren
2024-11-11 19:55 ` Johannes Schindelin
2024-11-10 20:30 ` Philippe Blain
2024-11-11 20:07 ` Johannes Schindelin
2024-11-26 7:58 ` Junio C Hamano
2024-11-11 0:37 ` Junio C Hamano
2024-11-11 16:51 ` Elijah Newren
2024-11-12 0:29 ` Junio C Hamano [this message]
2024-12-16 14:11 ` [PATCH v3 0/2] Support diff merges option in range diff Johannes Schindelin via GitGitGadget
2024-12-16 14:11 ` [PATCH v3 1/2] range-diff: optionally include merge commits' diffs in the analysis Johannes Schindelin via GitGitGadget
2024-12-16 14:11 ` [PATCH v3 2/2] range-diff: introduce the convenience option `--remerge-diff` Johannes Schindelin via GitGitGadget
2024-11-08 15:33 ` [PATCH] range-diff: optionally include merge commits' diffs in the analysis Elijah Newren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqwmh98fcg.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=j6t@kdbg.org \
--cc=johannes.schindelin@gmx.de \
--cc=levraiphilippeblain@gmail.com \
--cc=newren@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).