All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Joel Klinghed <the_jk@spawned.biz>
Cc: Joel Klinghed via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] commit: restore --edit when combined with --fixup
Date: Wed, 11 Aug 2021 18:22:49 -0400	[thread overview]
Message-ID: <YRRNuQWkgWxQyfU0@coredump.intra.peff.net> (raw)
In-Reply-To: <127626db-302e-426c-a158-35f46205e1d6@www.fastmail.com>

On Thu, Aug 12, 2021 at 12:10:28AM +0200, Joel Klinghed wrote:

> > Is the goal that they might leave notes for themselves, which they can
> > view in the meantime before they run "rebase --autosquash"?
> > 
> 
> At my work we use "fixup!" when pushing fixes to a review, using
> the modified body to outline what issue the commit is addressing,
> helping the reviewers to see intent and not just the end result.
> All "fixup!" are then ofc. squashed before integration.
> Same can be achieved with -m but --edit is generally
> easier to work with in my experience.
> 
> I've also once or twice used it to avoid a "fixup!" of a "fixup!" instead
> of looking up the original target commit hash but that's just me being
> lazy.

Ah, thanks for explaining. That makes sense.

> I'll add a test to check for previous behavior.

This is what I worked up, in case it helps.

diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
index 38a532d81c..3e4364066f 100755
--- a/t/t7502-commit-porcelain.sh
+++ b/t/t7502-commit-porcelain.sh
@@ -940,4 +940,22 @@ EOF
 	)
 '
 
+test_expect_success 'commit --fixup respects --edit' '
+	echo broken >foo.c &&
+	git add foo.c &&
+	git commit -m "wip of foo.c" &&
+	echo fixed >foo.c &&
+	(
+		test_set_editor "$TEST_DIRECTORY/t7500/add-content" &&
+		git commit --fixup HEAD --edit foo.c
+	) &&
+	cat >expect <<-\EOF &&
+	fixup! wip of foo.c
+
+	commit message
+	EOF
+	git log -1 --pretty=format:%B HEAD >actual &&
+	test_cmp expect actual
+'
+
 test_done

-Peff

  reply	other threads:[~2021-08-11 22:22 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-11 13:49 [PATCH] commit: restore --edit when combined with --fixup Joel Klinghed via GitGitGadget
2021-08-11 20:24 ` Jeff King
2021-08-11 22:10   ` Joel Klinghed
2021-08-11 22:22     ` Jeff King [this message]
2021-08-11 23:27     ` brian m. carlson
2021-08-11 23:43 ` [PATCH v2] " Joel Klinghed via GitGitGadget
2021-08-12  5:21   ` Junio C Hamano
2021-08-12  7:42     ` Joel Klinghed
2021-08-12 19:51       ` Junio C Hamano
2021-08-12  8:02   ` [PATCH v3] " Joel Klinghed via GitGitGadget
2021-08-12  9:32     ` Phillip Wood
2021-08-12 10:01       ` Joel Klinghed
2021-08-13 13:06         ` Phillip Wood
2021-08-13 15:35           ` Joel Klinghed
2021-08-14 15:20             ` Phillip Wood
2021-08-14 21:19               ` Joel Klinghed
2021-08-12 11:55     ` [PATCH v4] " Joel Klinghed via GitGitGadget
2021-08-14 21:40       ` [PATCH v5] " Joel Klinghed via GitGitGadget
2021-08-17 10:06         ` Phillip Wood

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=YRRNuQWkgWxQyfU0@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=the_jk@spawned.biz \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.