Re: Re* [PATCH 1/8] remote-bzr: fix export of utf-8 authors
On 08/29/2013 11:24 PM, Junio C Hamano wrote:
Show 25 quoted lines
> 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:
Show 13 quoted lines
>
> * 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.
Show 9 quoted lines
> 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/
Michael Haggerty <mhagger@alum.mit.edu>