All of lore.kernel.org
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Eric Sunshine via GitGitGadget <gitgitgadget@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH 0/3] fix failing t4301 test and &&-chain breakage
Date: Mon, 29 Aug 2022 19:52:59 -0700	[thread overview]
Message-ID: <CABPp-BGBEfFh0z0YHGcHE+rva9JdapXprPn-RSmif0xn6fcxYw@mail.gmail.com> (raw)
In-Reply-To: <pull.1339.git.1661663879.gitgitgadget@gmail.com>

On Sat, Aug 27, 2022 at 10:18 PM Eric Sunshine via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> This series fixes a failing test in t4301 due to 'sed' behavioral
> differences between implementations. It also fixes a couple broken &&-chains
> and adds missing explicit loop termination.
>
> The third patch is entirely subjective and can be dropped if unwanted. I
> spent more than a few minutes puzzling over the script's use of 'printf
> "\\n"' rather than the more typical 'printf "\n"' or even a simple 'echo',
> wondering if there was some subtlety I was missing or whether Elijah had
> encountered an unusual situation in which '\\n' was needed over '\n'. The
> third patch chooses to replace 'printf "\\n"' with 'echo' which I find more
> idiomatic, but I can see value in using 'printf "\n"' as perhaps being
> clearer that it is adding a newline where one is missing.

I can't actually provide the reasoning for it; I took Dscho's testcase
from [1] and used it as a basis for adding several other testcases.
When I was copying & pasting and adjusting, I just didn't notice the
'printf "\\n"'.  But using a simple echo makes sense.

[1] https://lore.kernel.org/git/3b4ed8bb1bb615277ee51a7b2af5fc53bae0a6e4.1660892256.git.gitgitgadget@gmail.com/

Anyway, I've read through the patches and your series looks good to me.

  parent reply	other threads:[~2022-08-30  2:53 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-28  5:17 [PATCH 0/3] fix failing t4301 test and &&-chain breakage Eric Sunshine via GitGitGadget
2022-08-28  5:17 ` [PATCH 1/3] t4301: account for behavior differences between sed implementations Eric Sunshine via GitGitGadget
2022-08-28  5:17 ` [PATCH 2/3] t4031: fix broken &&-chains and add missing loop termination Eric Sunshine via GitGitGadget
2022-08-28  5:17 ` [PATCH 3/3] t4301: emit blank line in more idiomatic fashion Eric Sunshine via GitGitGadget
2022-08-28 20:05 ` [PATCH 0/3] fix failing t4301 test and &&-chain breakage Junio C Hamano
2022-08-28 20:46   ` Eric Sunshine
2022-08-29  5:36     ` Junio C Hamano
2022-08-30  2:53   ` Elijah Newren
2022-08-30  2:56     ` Eric Sunshine
2022-08-30  2:59       ` Elijah Newren
2022-08-30  2:52 ` Elijah Newren [this message]
2022-08-30 14:02   ` Johannes Schindelin

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=CABPp-BGBEfFh0z0YHGcHE+rva9JdapXprPn-RSmif0xn6fcxYw@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=sunshine@sunshineco.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 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.