Re: [PATCH v2 06/10] t7006: add tests for how git tag paginates

["Martin Ågren" <martin.agren@gmail.com>]

On 31 July 2017 at 18:37, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> But here...
>>
>>> +test_expect_success TTY 'git tag -a respects pager.tag' '
>>> +    test_when_finished "git tag -d newtag" &&
>>> +    rm -f paginated.out &&
>>> +    test_terminal git -c pager.tag tag -am message newtag &&
>>> +    test -e paginated.out
>>> +'
>>
>> I think this behavior is just buggy, and it might be better introduced
>> as a test_expect_failure on "git tag -a does not respect pager.tag".
>>
>> Kind of a minor nit, as the series should end up in the right place
>> either way, but it can be helpful if you end up digging back in history
>> to the introduction of the test.
>
> Yes, I think that is essentially the same reaction I had to patches
> 7 and 8, where it carries the "buggy" behaviour forward and then
> fixes it.  The way the series lays groundwork to introduce a
> mechanism that can be used to address this behaviour in its earlier
> patches strongly suggests to the users that this is considered a bug
> by the author of the series to the user from early on, so adding
> this as "expect failure" and then flip it to "expect success" when
> the bug is fixed would be a more natural sequence of changes.

Thanks both for very helpful comments. I admit I viewed it less as
"fix buggy behavior" and more like "redefine wanted behavior". So I
wanted to postpone the redefinition of the behavior until all the
restructuring was done. Looking at this as a bug-fix does make
carefully moving the bug forward look rather silly.

I haven't responded to each of your suggestions individually where the
answers would have been a mere "thanks, will do". They're still much
appreciated and will help make v3 much better. Thanks.

Martin