Re: [PATCH 9/9] doc/diff-options: explain the new --remerge-diff option

On Tue, Dec 21, 2021 at 5:06 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

On Tue, Dec 21 2021, Elijah Newren wrote:

quoted

On Tue, Dec 21, 2021 at 1:29 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

quoted

On Tue, Dec 21 2021, Elijah Newren via GitGitGadget wrote:

quoted

From: Elijah Newren <redacted>

Signed-off-by: Elijah Newren <redacted>
---
 Documentation/diff-options.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index c89d530d3d1..b05f1c9f1c9 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt

@@ -64,6 +64,14 @@ ifdef::git-log[]
      each of the parents. Separate log entry and diff is generated
      for each parent.
 +
+--diff-merges=remerge:::
+--diff-merges=r:::
+--remerge-diff:::
+     With this option, two-parent merge commits are remerged to
+     create a temporary tree object -- potentially containing files
+     with conflict markers and such.  A diff is then shown between
+     that temporary tree and the actual merge commit.
++
 --diff-merges=combined:::
 --diff-merges=c:::
 -c:::

This & 5/9 would I think be better squashed into their respective "main"
patches.

I presume you mean the "main" patch for this one is 8/9.  I was trying
to find a way to break up that large patch, but this is pretty small
so...sure I'll squash it in.

What are you referring to as the "main" patch for 5/9, though?  It
only seems related to 6/9 and 7/9 to me, but I very deliberately split
those patches off and don't want to confuse them with unrelated
changes.  I disagree with combining 5/9 with either of those.

I just gave it a quick initial skim.

I have sometimes found it a bit harder to review your patches due to
over-splitting.

E.g. (went back and looked) here tmp_objdir_discard_objects() is
introduced in 1/9 but used in 8/9. "path_messages" is then introduced in
5/9 and used in 8/9, no?

Anyway, just a bit of feedback. FWIW not just bikeshedding. I do find
myself stopping at 1/9, paging to 2/9, searching for the function, not
there, checking 3/9 etc.

I realize this is a bit of a stones & glass houses comment, but I find
it a bit easier to review things when a patch is larger v.s. having it
split up in a way where preceding steps don't do anything yet except
wait for use by a subsequent patch.

0.02 etc.

Oh, 8/9.  That one could make sense.

And thanks for the feedback.  Perhaps I could restructure this series
with a top-down design instead of bottom-up.  Doing that would mean
either adding functions with an instant-die() implementation in the
first step or just leaving a placeholder comment, and then filling
those things in for later steps.