Re: [PATCH 1/3] t7001: reformat to newer style

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

Derrick Stolee <stolee@gmail.com> writes:

>> +test_expect_success 'moving the file back into subdirectory' '
>> +	cd path0 && git mv ../path1/COPYING COPYING
>> +'
>
> Split at &&, use subshell?

Yes, I was almost going to point out the same, saying "'reformat to
newer style' is much larger than only changing how the test body is
formatted", but was debating myself, as a good "modernization patch"
needs both mechanical changes and manual/semantic clean-ups, and it
is very clear that these three patches deliberately limit themselves
to the former for easier verification.

It is relatively rare that files are not touched by any in-flight
topic in the codebase, which is a good opportunity to apply this
kind of wholesale clean-up, so I tend to agree that it is a shame
not to do the non-mechanical clean up in the same series.  Perhaps
the best way would be to keep these three mechanical steps as they
are, and then follow-up with non-mechanical clean-up like you
suggested.

>> +test_expect_success 'commiting the change' '
>> +	cd .. && git commit -m move-in -a
>> +'
>
> Drop "cd .." (and the comments about being in path0)

... when the previous step moves to "git -C path0 mv ..." or
preferrably "(cd path0 && git mv ...)".


> [big snip]
>
>> +test_expect_success 'moving to existing tracked target with trailing slash' '
>> +	mkdir path2 &&
>> +	>path2/file && git add path2/file &&
> This line in particular looks a bit strange. What is this doing? At
> least we should split the &&.

Create an empty file by redirecting the output from a no-op into it,
and then adding the result.  I agree with you that this would be
easier to read on two lines.

>> +	git mv path1/path0/ path2/ &&
>> +	test_path_is_dir path2/path0/
>> +'