Re: [PATCH v3 1/2] t5100-mailinfo: replace common path prefix with variable

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 26, 2016, 19:06 UTC
Message-ID: <xmqqd1jqscp7.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160925210808.26424-1-me@ikke.info>

Kevin Daudt <me@ikke.info> writes:

Show 12 quoted lines

> Many tests need to store data in a file, and repeat the same pattern to
> refer to that path:
>
>     "$TEST_DIRECTORY"/t5100/
>
> Create a variable that contains this path, and use that instead.
>
> Signed-off-by: Kevin Daudt <me@ikke.info>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Changes since v2:
>  - changed $DATA to $data to indicate it's a script-local variable

If you are rerolling anyway, I would have liked to see the "why is only the variable part quoted?" issue addressed which was raised during the previous round of the review. I may have said it is OK to leave it as a low-hanging fruit for others but that only meant that it alone is not a strong enough reason to reroll this patch.

Other than that, looks good to me, though ;-)