From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Sun, 01 Sep 2013 04:03:22 GMT
Subject: Re: Re* [PATCH 1/8] remote-bzr: fix export of utf-8 authors
Message-ID: <5222BC8A.3020605@alum.mit.edu>
In-Reply-To: <xmqqhae82nsb.fsf_-_@gitster.dls.corp.google.com>

On 08/29/2013 11:24 PM, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>
>>> Do I really need to quote the paragraph in CodingGuidelines?
>>
>> Existing violations are not an excuse to make things worse by adding
>> more.  I think with these comments we can expect a reroll coming,
>> and it should be trivial for any contributor to fix it while at it.
> 
> Actually, I think not fixing it inside that 1/8 is good, as there
> are many existing "cmd > file" (and worse, "cmd > file-$x") in these
> test-*.sh scripts.  Clean-up is better done as a follow-up patch.
> 
> Here are two that I noticed.
> 
> -- >8 --
> Subject: [PATCH 9/8] contrib/remote-helpers: style updates for test scripts
> 
> During the review of the main series it was noticed that these test
> scripts can use updates to conform to our coding style better, but
> fixing the style should be done in a patch separate from the main
> series.
> 
> This updates the test-*.sh scripts only for styles:

s/styles/style/

> 
>  * We do not leave SP between a redirection operator and the
>    filename;
> 
>  * We change line before "then", "do", etc. rather than terminating
>    the condition for "if"/"while" and list for "for" with a
>    semicolon;
> 
>  * When HERE document does not use any expansion, we quote the end
>    marker (e.g. "cat <<\EOF" not "cat <<EOF") to signal the readers
>    that there is no funny substitution to worry about when reading
>    the code.
> 

Please add

    * We use "test" rather than "[".

, as you made a few such changes as well.

> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  contrib/remote-helpers/test-bzr.sh       | 112 +++++++++++-----------
>  contrib/remote-helpers/test-hg-bidi.sh   |  52 ++++++-----
>  contrib/remote-helpers/test-hg-hg-git.sh | 156 +++++++++++++++++--------------
>  contrib/remote-helpers/test-hg.sh        | 154 +++++++++++++++---------------
>  4 files changed, 248 insertions(+), 226 deletions(-)
> 
> [...]

All of the changes in the patch look good to me.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/