From: Jeff King <firstname.lastname@example.org> To: Eric Sunshine <email@example.com> Cc: Taylor Blau <firstname.lastname@example.org>, Git List <email@example.com>, Derrick Stolee <firstname.lastname@example.org> Subject: Re: [PATCH 2/3] t4216: fix broken '&&'-chain Date: Tue, 30 Jun 2020 14:55:36 -0400 [thread overview] Message-ID: <20200630185536.GA1888406@coredump.intra.peff.net> (raw) In-Reply-To: <CAPig+cQv3cHe5ci3mDvNKYXbVQt6Rp5icG-woaDCqfAtzZ6SZw@mail.gmail.com> On Tue, Jun 30, 2020 at 01:50:22PM -0400, Eric Sunshine wrote: > > diff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh > > @@ -53,7 +53,7 @@ sane_unset GIT_TRACE2_PERF_BRIEF > > sane_unset GIT_TRACE2_CONFIG_PARAMS > > Not related to this patch, but 'sane_unset' is pointless outside of a > test since there is no &&-chain to maintain. Plain 'unset' would work > just as well and be less misleading. We do this in lots of other places, too. Try: git grep sane_unset | grep -v '&&' Though seeing the variables they cover, I think many of them may come from the same few authors. :) I wonder if it is worth keeping these as sane_unset, though. Your comment is based on the knowledge that the "sane" part of the function is ignoring the return value. But we could conceivably have other portability fixes (it's not impossible that a shell wants one "unset" per variable, for example), in which case we'd want it in more places. That's hypothetical, of course, but saying "just use our portable unset wrapper that behaves sanely" seems like exactly what these call sites want, now and in a hypothetical future. -Peff
next prev parent reply other threads:[~2020-06-30 18:55 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-06-30 17:17 [PATCH 0/3] commit-graph: introduce 'core.useBloomFilters' Taylor Blau 2020-06-30 17:17 ` [PATCH 1/3] commit-graph: pass a 'struct repository *' in more places Taylor Blau 2020-06-30 20:52 ` Derrick Stolee 2020-06-30 17:17 ` [PATCH 2/3] t4216: fix broken '&&'-chain Taylor Blau 2020-06-30 17:50 ` Eric Sunshine 2020-06-30 18:39 ` Taylor Blau 2020-06-30 19:03 ` Jeff King 2020-06-30 19:12 ` Taylor Blau 2020-06-30 19:19 ` Jeff King 2020-06-30 19:48 ` Eric Sunshine 2020-06-30 18:55 ` Jeff King [this message] 2020-06-30 17:17 ` [PATCH 3/3] commit-graph: respect 'core.useBloomFilters' Taylor Blau 2020-06-30 19:18 ` Jeff King 2020-06-30 19:27 ` Taylor Blau 2020-06-30 19:33 ` Jeff King 2020-08-03 19:02 ` [PATCH 0/3] commit-graph: introduce 'core.useBloomFilters' Taylor Blau
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 List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200630185536.GA1888406@coredump.intra.peff.net \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH 2/3] t4216: fix broken '\''&&'\''-chain' \ /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
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git 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 read-only IMAP folder(s) and NNTP newsgroup(s).