On 31 July 2017 at 18:37, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Jeff King <peff@xxxxxxxx> 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
