Re: [PATCH 1/2] t0001: remove pipes

From: Shubham Mishra <shivam828787@gmail.com>
Date: Feb 22, 2022, 18:16 UTC
Message-ID: <CAC316V5CcB_UxpVYdAYZDPpY_ebauu4K23TrU1zLnTdgXg0PNQ@mail.gmail.com>
In-Reply-To: <9d08d2a6-82ed-7d6c-6a82-0e89c55bd94a@github.com>

Show 5 quoted lines

> > pipes doesn't care about error codes and ignore them thus we should not use them in tests.
> > As an easy alternative, I am using a tmp file to write from git command so we can test the exit code.
>
> Please be careful about the length of your lines in your
> commit message. vim should auto-wrap as you write.

Oops, I mostly use Nano, I think it's time to use vim. I will take care of it.

Show 6 quoted lines

> There are also some grammar issues, so here is an update
> to what you wrote:
>
>   Pipes ignore error codes and thus we should not use them
>   in tests. As an alternative, use a 'tmp' file to write the
>   Git output so we can test the exit code.

Thanks for pointing this out. I will update it. I am a non-native English speaker, and looking to improve my command in language with time :)

Show 15 quoted lines

> Split each part of the &&-chain across lines, like this:
>
> +               find .git/worktrees -print >tmp &&
> +               sort tmp >expected &&
>
> >               git -C ../linked-worktree init &&
> >               test_cmp expected-exclude .git/info/exclude &&
> >               test_cmp expected-config .git/config &&
> > -             find .git/worktrees -print | sort >actual &&
> > +             find .git/worktrees -print >tmp && sort tmp >actual &&
>
> Here, too.
>
> I could make the same comments on patch 2, but I think you
> have enough info to go on.

Sure, I get it! I will consider all these suggestions in v2.

Thanks, Shubham