Re: [PATCH v8 7/7] list-objects-filter: implement filter tree:0

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

>> diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
>> index bbbe7537d..8eeb85fbc 100755
>> --- a/t/t5616-partial-clone.sh
>> +++ b/t/t5616-partial-clone.sh
>> ...
>
> Break line after pipe "|", not before, and lose the backslash.  You
> do not need to over-indent the command on the downstream of the
> pipe, i.e.
>
> 	awk ... |
> 	xargs -n1 git -C ... &&
>
> Same comment applies elsewhere in this patch, not limited to this file.
>
>> +	sort fetched_types -u >unique_types.observed &&
>
> Make it a habit not to add dashed options after real arguments, i.e.
>
> 	sort -u fetched_types
>
>> +	echo commit >unique_types.expected &&
>> +	test_cmp unique_types.observed unique_types.expected &&
>
> Always compare "expect" with "actual", not in the reverse order, i.e.
>
> 	test_cmp expect actual
>
> not
>
> 	test_cmp actual expect
>
> This is important because test_cmp reports failures by showing you
> an output of "diff expect actual" and from "sh t5616-part*.sh -v"
> you can see what additional/excess things were produced by the test
> over what is expected, prefixed with "+", and what your code failed
> to produce are shown prefixed with "-".

I notice that patches to other files like 6112 in this series also
spread the above mistakes from existing lines.  Please do not view
what you see in these two test scripts before you start touching as
a good example to follow---rather, treat them as antipattern X-<.
5616 is not as bad as 6112, but they both need to be cleaned up.

We could alternatively do a post clean-up, but ideally we should
first have a clean-up patch before this series to co.

Thanks.