From: Jeff King <peff@peff•net>
To: Aaron Plattner <aplattner@nvidia•com>
Cc: Junio C Hamano <gitster@pobox•com>,
git@vger•kernel.org, Patrick Steinhardt <ps@pks•im>,
Rahul Rameshbabu <rrameshbabu@nvidia•com>,
Lucas De Marchi <demarchi@kernel•org>
Subject: Re: [PATCH v2] remote-curl: Use auth for probe_rpc() requests too
Date: Wed, 14 Jan 2026 12:30:55 -0500 [thread overview]
Message-ID: <20260114173055.GD885771@coredump.intra.peff.net> (raw)
In-Reply-To: <613b47af-5269-44e3-87f5-d29fba9d73b3@nvidia.com>
On Wed, Jan 14, 2026 at 08:33:43AM -0800, Aaron Plattner wrote:
> Ooh, neat. I guess I copied the wrong example. I verified that this works
> too, so I can send a v5 for that.
>
> Is it worth changing up the other cases of this pattern, mostly in
> pack-refs-tests.sh? E.g.,
>
> # Create 15 loose references.
> printf "create refs/heads/loose-%d HEAD\n" $(test_seq 15) >stdin &&
> git update-ref --stdin <stdin &&
>
> [...]
>
> # Create 99 packed refs. This should cause the heuristic
> # to require more than the minimum amount of loose refs.
> test_seq 99 |
> while read i
> do
> printf "create refs/heads/packed-%d HEAD\n" $i || return 1
> done >stdin &&
> git update-ref --stdin <stdin &&
>
> I can put together a patch for those.
Yeah, I think they are worth updating. I looked for spots to convert
when I added the feature in b32c7ec02f (test-lib: teach test_seq the -f
option, 2025-06-23). But I missed those ones.
I think I grepped for "for i in $(test_seq ...)", but the use of the
while loop and the $()-substitution meant I didn't see them. Examining
every test_seq call didn't seem worthwhile, as there are hundreds. ;)
Grepping for "test_seq .* |" does yield a few more (e.g., p5551), but
most are false positives. Grepping for "printf.*$(test_seq" gets some
more.
I don't know that we need to exhaustively find all cases. ;) In the
first case above, it does save us a subshell. In the second case I think
it saves us a subshell _and_ the result is much easier to read (because
it avoids the loop and return). So IMHO it's a nice improvement, but
there's diminishing returns to investigating every test_seq call.
-Peff
next prev parent reply other threads:[~2026-01-14 17:30 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-13 3:19 [PATCH v2] remote-curl: Use auth for probe_rpc() requests too Aaron Plattner
2026-01-13 6:49 ` Patrick Steinhardt
2026-01-13 13:09 ` Junio C Hamano
2026-01-14 1:06 ` Aaron Plattner
2026-01-14 2:20 ` Jeff King
2026-01-14 14:11 ` Junio C Hamano
2026-01-14 16:33 ` Aaron Plattner
2026-01-14 17:30 ` Jeff King [this message]
2026-01-14 17:41 ` Junio C Hamano
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=20260114173055.GD885771@coredump.intra.peff.net \
--to=peff@peff$(echo .)net \
--cc=aplattner@nvidia$(echo .)com \
--cc=demarchi@kernel$(echo .)org \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=ps@pks$(echo .)im \
--cc=rrameshbabu@nvidia$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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