git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] t2200: modify code syntax

From
Amanda Shafack <shafack.likhene@gmail.com>
Date
Oct 18, 2020, 13:59 UTC
Message-ID
<CAGxm6oXhW8x2jkN-P4emhp6BvAwk6urzFTauQBDhJdMV_0d1fg@mail.gmail.com>
In-Reply-To
<CAPig+cRuutHJ64aXQy5X9h-Vg4=_NKKyL57KdwbvXj4cGRS1PQ@mail.gmail.com>

I agree with you. I have made these changes as well. Thanks

On Sun, Oct 18, 2020 at 6:56 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 47 quoted lines
>
> On Sat, Oct 17, 2020 at 12:02 PM Amanda Shafack via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > Using the logical not operator on both the git and grep command is
> > redundant, since the main check is done at the level of the grep
> > command.
> >
> > Apply the logical not operator only to the grep command.
> > ---
> > diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
> > @@ -179,10 +179,8 @@ test_expect_success 'add -u resolves unmerged paths' '
> > -       ! (
> > -               git ls-files >actual &&
> > -               grep "non-existent" actual
> > -       )
> > +       git ls-files >actual &&
> > +       ! grep "non-existent" actual
>
> The commit message talks only about redundancy of applying the logical
> NOT operator to the combined subshell content, but what it misses is
> that transformation of:
>
>     ! (git ls-files | grep "non-existent")
>
> in patch [1/2] into:
>
>     ! (
>         git ls-files >actual &&
>         grep "non-existent" actual
>     )
>
> is actively wrong. In particular, if `git ls-files` itself fails, then
> the `grep` will never get run, and the subshell will exit with a
> failure, however, the NOT then turns that failure into a success,
> which is quite undesirable. The test should fail if either `git
> ls-files` fails or if the `grep` expectation is not met; it should not
> succeed when `git ls-files` fails.
>
> So, the correct thing to do is to merge [1/2] and [2/2] into a single
> patch which directly transforms:
>
>     ! (git ls-files | grep "non-existent")
>
> into:
>
>     git ls-files >actual &&
>     ! grep "non-existent" actual
-- 
Cheers!

Amanda  Shafack
Previous: Eric SunshineNext: Amanda Shafack via GitGitGadget
Message 4 of 14 in “t9832,t2200: avoid using pipes in git related commands”
  1. 0/2 [Outreachy][Patch v1] t9832,t2200: avoid using pipes in git related commandsAmanda Shafack via GitGitGadget, Oct 17, 2020
  2. 2/2 t2200: modify code syntaxAmanda Shafack via GitGitGadget, Oct 17, 2020
  3. Eric SunshineOct 18, 2020
  4. Amanda ShafackOct 18, 2020
  5. 1/2 t9832,t2200: avoid using pipes in git commandsAmanda Shafack via GitGitGadget, Oct 17, 2020
  6. Eric SunshineOct 18, 2020
  7. Amanda ShafackOct 18, 2020
  8. t9832,t2200: avoid using pipes in git commandsAmanda Shafack via GitGitGadget, Oct 18, 2020
  9. Eric SunshineOct 18, 2020
  10. Junio C HamanoOct 18, 2020
  11. Amanda ShafackOct 18, 2020
  12. Amanda ShafackOct 18, 2020
  13. t2200,t9832: avoid using 'git' upstream in a pipeAmanda Shafack via GitGitGadget, Oct 18, 2020
  14. Junio C HamanoOct 18, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.