All of lore.kernel.org
 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
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.