From: Junio C Hamano <gitster@pobox.com>
To: Elijah Newren <newren@gmail.com>
Cc: Christian Couder <christian.couder@gmail.com>,
git@vger.kernel.org, Patrick Steinhardt <ps@pks.im>,
Jeff King <peff@peff.net>,
"brian m . carlson" <sandals@crustytoothpaste.net>,
Johannes Schindelin <Johannes.Schindelin@gmx.de>,
Christian Couder <chriscool@tuxfamily.org>
Subject: Re: [PATCH v6] fast-(import|export): improve on commit signature output format
Date: Mon, 14 Jul 2025 14:23:26 -0700 [thread overview]
Message-ID: <xmqqikjulb5d.fsf@gitster.g> (raw)
In-Reply-To: <CABPp-BHM5afgiUf7GsTPWmrf_tm6mWnvHWMKiZPxApJzN-U8gg@mail.gmail.com> (Elijah Newren's message of "Mon, 14 Jul 2025 14:07:14 -0700")
Elijah Newren <newren@gmail.com> writes:
> Overall, the patch looks great now. There's just one little nit-pick
> left; I'm still not a fan of tests of the form
>
> (
> cd dir &&
> git fast-import
> ...lots of other commands...
> ) <output
>
> because I think the "<output" should really be moved to the "git
> fast-import" line since it's only meant to be used there.
>
> This series adds 2 such tests. You did point out in the discussion on
> v5 that the testsuite already uses this idiom and you wanted to match
> existing style. (Though there were only 2 tests previously that used
> it, and you already modified both as part of this patch.)
>
> However...we've been through enough rounds and this is really just a
> nit-pick; I can submit a follow-on patch later to clean up the four
> tests and see if others agree with me that this is an eyesore, or if
> I'm just weird.
FWIW, I think it makes sense to ensure that the "output" is consumed
only by the intended command. And "there are already two cases"
would not work very well as an excuse to add two more to make the
codebase even worse.
> I think it's good to merge down.
OK. As long as somebody promises that the result will be cleaned up
soon later, I am OK with that.
Thanks.
next prev parent reply other threads:[~2025-07-14 21:23 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-24 20:39 [PATCH] fast-(import|export): improve on the signature algorithm name Christian Couder
2025-04-24 21:19 ` Junio C Hamano
2025-04-24 21:59 ` Elijah Newren
2025-04-24 22:58 ` Junio C Hamano
2025-05-26 10:35 ` Christian Couder
2025-05-27 15:18 ` Junio C Hamano
2025-05-28 17:29 ` Junio C Hamano
2025-05-28 20:06 ` Elijah Newren
2025-05-28 21:59 ` Junio C Hamano
2025-05-28 23:15 ` Elijah Newren
2025-05-29 3:14 ` Junio C Hamano
2025-06-02 15:56 ` Christian Couder
2025-06-02 15:56 ` Christian Couder
2025-06-02 16:20 ` Junio C Hamano
2025-05-26 10:34 ` Christian Couder
2025-04-24 21:41 ` Elijah Newren
2025-05-26 10:34 ` Christian Couder
2025-04-24 22:05 ` brian m. carlson
2025-05-26 10:35 ` Christian Couder
2025-04-24 23:25 ` Junio C Hamano
2025-05-26 10:33 ` [PATCH v2 0/6] extract algo information from signatures Christian Couder
2025-05-26 10:33 ` [PATCH v2 1/6] gpg-interface: simplify ssh fingerprint parsing Christian Couder
2025-05-26 10:33 ` [PATCH v2 2/6] gpg-interface: use left shift to define GPG_VERIFY_* Christian Couder
2025-05-26 10:33 ` [PATCH v2 3/6] doc/verify-commit: update and improve the whole doc Christian Couder
2025-05-26 10:33 ` [PATCH v2 4/6] gpg-interface: extract hash algorithm from signature status output Christian Couder
2025-05-26 10:33 ` [PATCH v2 5/6] gpg-interface: extract SSH key type " Christian Couder
2025-05-26 10:33 ` [PATCH v2 6/6] verify-commit: add a --summary flag Christian Couder
2025-05-26 16:03 ` [PATCH v2 0/6] extract algo information from signatures Elijah Newren
2025-06-19 13:38 ` Christian Couder
2025-06-02 22:17 ` brian m. carlson
2025-06-19 13:37 ` Christian Couder
2025-06-18 15:18 ` [PATCH v3] fast-(import|export): improve on commit signature output format Christian Couder
2025-06-19 13:36 ` [PATCH v4] " Christian Couder
2025-06-19 14:55 ` Junio C Hamano
2025-07-08 9:16 ` Christian Couder
2025-06-19 21:44 ` Elijah Newren
2025-06-20 16:12 ` Christian Couder
2025-06-20 19:20 ` Junio C Hamano
2025-07-08 9:16 ` Christian Couder
2025-06-26 19:11 ` Elijah Newren
2025-07-08 9:16 ` Christian Couder
2025-07-07 22:58 ` Junio C Hamano
2025-07-08 3:35 ` Christian Couder
2025-07-08 5:03 ` Junio C Hamano
2025-07-08 6:38 ` Patrick Steinhardt
2025-07-08 11:08 ` Christian Couder
2025-07-08 16:38 ` Junio C Hamano
2025-07-09 0:19 ` Christian Couder
2025-07-09 15:35 ` Junio C Hamano
2025-07-10 8:25 ` Patrick Steinhardt
2025-07-10 15:29 ` Christian Couder
2025-07-10 15:33 ` Junio C Hamano
2025-07-08 10:17 ` Christian Couder
2025-07-08 9:17 ` [PATCH v5] " Christian Couder
2025-07-08 21:58 ` Junio C Hamano
2025-07-08 23:08 ` Elijah Newren
2025-07-09 0:03 ` Junio C Hamano
2025-07-09 0:10 ` Elijah Newren
2025-07-09 10:18 ` Christian Couder
2025-07-09 10:15 ` Christian Couder
2025-07-09 14:12 ` [PATCH v6] " Christian Couder
2025-07-09 23:14 ` Junio C Hamano
2025-07-14 21:07 ` Elijah Newren
2025-07-14 21:23 ` Junio C Hamano [this message]
2025-07-25 16:11 ` Christian Couder
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=xmqqikjulb5d.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=chriscool@tuxfamily.org \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=newren@gmail.com \
--cc=peff@peff.net \
--cc=ps@pks.im \
--cc=sandals@crustytoothpaste.net \
/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).