Re: [PATCH 3/3] t/t4013: add test for --diff-merges=off

Junio C Hamano [off-list ref] writes:

Junio C Hamano [off-list ref] writes:

quoted

quoted

Dunno, why original Jeff series didn't need 

     log --first-parent --no-diff-merges -p master

then?

Who said it didn't need it?

To elaborate a bit more, we are all humans, and even reviewers
should be given the second chance to suggest improvements to things
that can use them, which s/he previously missed.  If we keep saying
"you say this is wrong, but the other guy did the same wrong thing
last week", we can never make the world better.

I asked because I thought you see some essential difference between two
tests, as you didn't suggest to add similar permutation test to the
original. I think this reply resolves my doubt.

I still think this additional test not needed unless we are going to
test all the permutations, and there are already 6 of them even for this
simple test, so we'd need to add 10 more tests to the 2 originals. If
somebody is willing to implement machinery to test all the option
permutations automatically... But that's out of scope of these patches.

To be honest, if we don't test all the permutations anyway, I'd rather
test most "human" variant only, that to me is:

  log --first-parent -p --no-diff-merges master

because --first-parent tells how to travel in history, and the rest
tells how to represent results, starting from generic "show me diffs"
and followed by "oh, but don't bother me with merge diffs, please".

And now we are back to my original suggestion to invert the order of
parameters in these tests, but approached from another direction ;)

Overall, I can obviously add such a test to the series if you insist,
but to me it still looks pointless. What doesn't look pointless is
replacing the original with the version you suggested, for the reasons
that I've already explained in preliminary discussions of these patches.

Finally, if you still insist on additional test(s), please also tell if
I should add similar permutation to the original --no-diff-merges test.

Thanks,
-- Sergey