Re: [GSoC] [PATCH v2 2/3] t9811: Remove the -q quiet mode from some instances of grep

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Apr 7, 2025, 16:26 UTC
Message-ID: <CAPig+cQbMUQAHsx92QNLAvfkUKRFoVoWznhPQok0PN6HW3wxzQ@mail.gmail.com>
In-Reply-To: <20250407111824.46518-3-anthonywang03@icloud.com>

On Mon, Apr 7, 2025 at 7:18 AM Anthony Wang <anthonywang513@gmail.com> wrote:

Show 5 quoted lines

> Remove the `-q` quiet mode from some instances of `grep`,
> as the lack of `-q` on the `grep` on line 99 implies that its output is
> required, when that is not the case. This change ensures consistency and
> avoids confusion about whether the output of `grep` is used.
> ---

Missing sign-off.

Rather than mentioning line 99 explicitly, it probably would be more helpful to readers to say instead:

    ... the lack of `-q` on the "TAG_F1" `grep` implies...

Show 8 quoted lines

> diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh
> @@ -97,8 +97,8 @@ test_expect_success 'two labels on the same changelist' '
>                 git tag >output &&
>                 grep TAG_F1 output &&
> -               grep -q TAG_F1_1 output &&
> -               grep -q TAG_F1_2 output &&
> +               grep TAG_F1_1 output &&
> +               grep TAG_F1_2 output &&

For such a simple change, it probably would be more common on this project to combine patches [2/3] (which drops `-q`) and [3/3] (which replaces `grep` with `test_grep`), and to simply explain as a side-note in the commit message of the combined patch why `-q` is being dropped. However, it's also fairly subjective, and others may feel differently, so keeping the changes as separate patches is also valid.