git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: Incremental updates to What's cooking
Date: Wed, 29 Feb 2012 14:50:42 +0100	[thread overview]
Message-ID: <4F4E2D32.9030209@in.waw.pl> (raw)
In-Reply-To: <7vbooiuj6z.fsf@alter.siamese.dyndns.org>

On 02/29/2012 09:39 AM, Junio C Hamano wrote:
> Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl>  writes:
>
>> On 02/28/2012 07:53 AM, Junio C Hamano wrote:
>>
>>> * zj/diff-stat-dyncol (2012-02-27) 11 commits
>>>    - diff --stat: add config option to limit graph width
>>>    - diff --stat: enable limiting of the graph part
>>>    - diff --stat: add a test for output with COLUMNS=40
>>>    - diff --stat: use a maximum of 5/8 for the filename part
>>>    - merge --stat: use the full terminal width
>>>    - log --stat: use the full terminal width
>>>    - show --stat: use the full terminal width
>>>    - diff --stat: use the full terminal width
>>>    - diff --stat: tests for long filenames and big change counts
>>>    - t4014: addtional format-patch test vectors
>>>    - Merge branches zj/decimal-width, zj/term-columns and jc/diff-stat-scaler
>>>
>>> I resurrected the additional tests for format-patch from an earlier round,
>>> as it illustrates the behaviour change brought by "5/8 split" very well.
>> Hi,
>> the resurrected tests are partly duplicated in 4052-stat-output.sh:
>>
>> t4014:
>> ok 75 - small change with long name gives more space to the name
>> ok 76 - a long name is given more room when the bar is short
>> ok 77 - format patch --stat-width=width works with long name        *
>> ok 78 - format patch --stat=...,name-width with long name           *
>> ok 79 - format patch --stat-name-width with long name               *
>> ok 81 - format patch graph part width                               *
>> ok 82 - format patch ignores COLUMNS                                *
>> ok 83 - format patch --stat=width with big change                   *
>> ok 84 - format patch --stat-width=width with big change             *
>> ok 85 - partition between long name and big change is more balanced
>>
>> t4052:
>> ok 3 - format-patch graph width defaults to 80 columns
>> ok 4 - format-patch --stat=width with long name
>> ok 5 - format-patch --stat-width=width with long name
>> ok 6 - format-patch --stat=...,name-width with long name
>> ok 7 - format-patch --stat-name-width with long name
>> ok 24 - format-patch ignores too many COLUMNS (big change)
>> ok 28 - format-patch ignores not enough COLUMNS (big change)
>> ok 29 - format-patch ignores statGraphWidth config
>> ok 36 - format-patch --stat=width with big change
>> ok 37 - format-patch --stat-width=width with big change
>> ok 38 - format-patch --stat-graph--width with big change
>> ok 49 - format-patch --stat=width with big change and long name
>> ok 53 - format-patch ignores COLUMNS (long filename)
>>
>> The ones with * are duplicated exactly. They tests run very fast, but
>> maybe the duplicated ones should be culled.
>
> Yeah, probably we should de-dup them.
>
> Compare the behaviour change shown for t4052 and for t4014 by 119c07bf.
> Which one more obviously show the effect of the code change to allow the
> reader judge if the behaviour change is going in a good direction?
t4014 it seems, but mostly because of more descriptive test names.
But this is only true for the ones without *, I think. So the ones with 
* can be deleted without losing this advantage.

The ones with * are very similar in t4014 and t4052.

> The style used in t4052 only changes expect_failure to expect_success, and
> the reader has to accept the judgement of the person who wrote the test
> vector and declared "this is the _right_ output!".  The way t4014, taken
> from your earlier round, shows the behaviour change shows how the
> expectation changes from the old behaviour to the new one, and the reader
> can see and decide which one is giving a better output.
>
> Actually, the whole reason I didn't notice duplicates in 4052 was because
> of the above X-<.
>
> If we remove duplicates, will 4052 become empty?  It would be really nice
> if we do not have to add a new test script for this series, and instead
> add necessary new tests to existing scripts.
t4052 tests show, log, merge, diff and format-patch with basically the 
same commands. Separating the tests into different files would require 
duplicating a lot of setup code. OTOH, t4014 is only about format-patch, 
so the other ones don't fit. I thought it would be better to create a 
new file.

-
Zbyszek

  reply	other threads:[~2012-02-29 13:50 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-28  6:53 Incremental updates to What's cooking Junio C Hamano
2012-02-29  7:37 ` Zbigniew Jędrzejewski-Szmek
2012-02-29  8:39   ` Junio C Hamano
2012-02-29 13:50     ` Zbigniew Jędrzejewski-Szmek [this message]
2012-02-29 19:28       ` Junio C Hamano
2012-02-29 20:19         ` Zbigniew Jędrzejewski-Szmek
2012-02-29 20:48           ` Junio C Hamano
2012-03-01 12:26             ` [PATCH v7a 1/9] diff --stat: tests for long filenames and big change counts Zbigniew Jędrzejewski-Szmek
2012-03-01 12:26               ` [PATCH v7a 2/9] diff --stat: use the full terminal width Zbigniew Jędrzejewski-Szmek
2012-03-01 12:26               ` [PATCH v7a 3/9] show " Zbigniew Jędrzejewski-Szmek
2012-03-01 12:26               ` [PATCH v7a 4/9] log " Zbigniew Jędrzejewski-Szmek
2012-03-01 12:26               ` [PATCH v7a 5/9] merge " Zbigniew Jędrzejewski-Szmek
2012-03-01 12:26               ` [PATCH v7a 6/9] diff --stat: use a maximum of 5/8 for the filename part Zbigniew Jędrzejewski-Szmek
2012-03-01 17:18                 ` Junio C Hamano
2012-03-26 23:45                 ` Jeff King
2012-03-27  5:10                   ` [PATCH] t4052: unset $COLUMNS inherited from environment Zbigniew Jędrzejewski-Szmek
2012-03-27  5:17                     ` Jeff King
2012-03-27  6:22                       ` [PATCH v2] tests: unset COLUMNS " Zbigniew Jędrzejewski-Szmek
2012-03-27 18:44                         ` Jeff King
2012-03-27  5:32                     ` [PATCH] t4052: unset $COLUMNS " Johannes Sixt
2012-03-01 12:26               ` [PATCH v7a 7/9] diff --stat: add a test for output with COLUMNS=40 Zbigniew Jędrzejewski-Szmek
2012-03-01 12:26               ` [PATCH v7a 8/9] diff --stat: enable limiting of the graph part Zbigniew Jędrzejewski-Szmek
2012-03-01 12:26               ` [PATCH v7a 9/9] diff --stat: add config option to limit graph width Zbigniew Jędrzejewski-Szmek
  -- strict thread matches above, loose matches on Subject: below --
2012-03-20  0:23 What's cooking in git.git (Mar 2012, #07; Mon, 19) Junio C Hamano
2012-03-20 23:35 ` Incremental updates to What's cooking Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F4E2D32.9030209@in.waw.pl \
    --to=zbyszek@in.waw.pl \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).