git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git List <git@vger.kernel.org>, Sangeeta <sangunb09@gmail.com>,
	Philippe Blain <levraiphilippeblain@gmail.com>,
	Kaartic Sivaraam <kaartic.sivaraam@gmail.com>,
	Christian Couder <christian.couder@gmail.com>,
	Lars Schneider <larsxschneider@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH] t/perf: fix test_export() failure with BSD `sed`
Date: Fri, 18 Dec 2020 01:15:05 -0500	[thread overview]
Message-ID: <CAPig+cQvaOBo=zx=f2ZsPy7QnPXgcdc0SUZyGWaZPZ31FUUwZg@mail.gmail.com> (raw)
In-Reply-To: <X9xBRXW7/tXsqLT5@coredump.intra.peff.net>

On Fri, Dec 18, 2020 at 12:44 AM Jeff King <peff@peff.net> wrote:
> On Wed, Dec 16, 2020 at 02:29:26PM -0500, Eric Sunshine wrote:
> > Perhaps a test_unexport() might be handy in the distant future, but
> > presently there is only a single call to test_export() in the entire
> > suite, so it's probably not worth worrying about now.
>
> I actually wonder if we could drop test_export entirely. I assume you
> mean the call in p0001. It is inside a test_expect_success block, where
> we don't need to do anything fancier than just "export". It is already
> running in the main script's environment, just like a normal test. If it
> were in a test_perf, then we would need to take special care to get it
> back into the main script.

Considering that test_export() hasn't seen much use since its
introduction nine years ago and that the one and only existing call
doesn't even need the special subprocess magic, retiring the function
is certainly an option. On the other hand, aside from this one minor
portability fix, it hasn't been a maintenance burden and may actually
come in handy someday if people start writing more "perf" tests. So, I
don't feel strongly one way or the other, though I lean somewhat
toward keeping it around.

  reply	other threads:[~2020-12-18  6:16 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-16  7:39 [PATCH] t/perf: fix test_export() failure with BSD `sed` Eric Sunshine
2020-12-16 19:07 ` Junio C Hamano
2020-12-16 19:29   ` Eric Sunshine
2020-12-18  5:42     ` Jeff King
2020-12-18  6:15       ` Eric Sunshine [this message]
2020-12-18  6:24         ` Jeff King
2020-12-21  7:23     ` Eric Sunshine
2020-12-20 21:27 ` [PATCH 2/1] t/perf: avoid unnecessary test_export() recursion Eric Sunshine

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='CAPig+cQvaOBo=zx=f2ZsPy7QnPXgcdc0SUZyGWaZPZ31FUUwZg@mail.gmail.com' \
    --to=sunshine@sunshineco.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=kaartic.sivaraam@gmail.com \
    --cc=larsxschneider@gmail.com \
    --cc=levraiphilippeblain@gmail.com \
    --cc=peff@peff.net \
    --cc=sangunb09@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).