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

Re: [PATCH] t*: avoid using pipes

From
Prathamesh Chavan <pc44800@gmail.com>
Date
Mar 8, 2017, 13:32 UTC
Message-ID
<CAME+mvUzR6--AeUff6yGZ69GN-hE6AyDP-CkdFxnFAwccpn2yg@mail.gmail.com>
In-Reply-To
<20170308060334.6ilcjgaxgycuhpxu@sigill.intra.peff.net>
On Wed, Mar 8, 2017 at 11:33 AM, Jeff King <peff@peff.net> wrote:
Show 34 quoted lines
> On Tue, Mar 07, 2017 at 12:52:49PM -0800, Stefan Beller wrote:
>
>> On Tue, Mar 7, 2017 at 12:39 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>>
>> > Welcome to the Git community!
>>
>> >
>> > Actually, being a *micro* project, it should stay so. Not doing all of the
>> > changes would leave some tasks for other apprentices to get warm with our
>> > review process.
>>
>> right, so just pick one file.
>
> I also wonder if we really want all invocations of git to be marked up
> in this way. If the primary goal of the test is checking that a certain
> git command runs successfully and generates the expected output, then I
> think it is a good candidate for conversion.
>
> So in a hunk like this:
>
>    test_expect_success 'git commit-tree records the correct tree in a commit' '
>         commit0=$(echo NO | git commit-tree $P) &&
>   -     tree=$(git show --pretty=raw $commit0 |
>   -              sed -n -e "s/^tree //p" -e "/^author /q") &&
>   +     tree=$(git show --pretty=raw $commit0 >out &&
>   +     sed -n -e "s/^tree //p" -e "/^author /q" <out) &&
>         test "z$tree" = "z$P"
>
> we are interested in testing commit-tree, not "git show". Is it worth
> avoiding pipes there? I admit the cost to using the intermediate file is
> not huge there, but it feels more awkward and un-shell-like to me as a
> reader.
>
> -Peff

Thank you everyone, for reviewing my changes. And as said in the reviews, I'll send a single patch file as my microproject, leaving the other files as low hanging fruit for the others to look at. Also, I try to include as many suggested improvements as possible and will also remember them for my future patches.

Previous: Jeff KingNext: Prathamesh Chavan
Message 6 of 7 in “t*: avoid using pipes”
  1. t*: avoid using pipesPrathamesh Chavan, Mar 7, 2017
  2. Stefan BellerMar 7, 2017
  3. Johannes SixtMar 7, 2017
  4. Stefan BellerMar 7, 2017
  5. Jeff KingMar 8, 2017
  6. Prathamesh ChavanMar 8, 2017
  7. Prathamesh ChavanMar 9, 2017

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.