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 9, 2017, 05:26 UTC
Message-ID
<CAME+mvWRSEJ6kp4MpELx20DkeR+pDED-KGB2o6yQ-26FmLvqWw@mail.gmail.com>
In-Reply-To
<CAME+mvUzR6--AeUff6yGZ69GN-hE6AyDP-CkdFxnFAwccpn2yg@mail.gmail.com>

I have created the required changes and submitted a single file patch. Also I tried my best to include each of the suggestions in that patch.

https://public-inbox.org/git/0102015aae7b8536-00c57d0a-1d48-4153-a202-87c4ea9e0e19-000000@eu-west-1.amazonses.com/
On Wed, Mar 8, 2017 at 7:02 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
Show 41 quoted lines
> On Wed, Mar 8, 2017 at 11:33 AM, Jeff King <peff@peff.net> wrote:
>> 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: Prathamesh Chavan
Message 7 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.