From: Michael Haggerty <mhagger@alum•mit.edu>
To: Junio C Hamano <gitster@pobox•com>
Cc: Matthieu Moy <Matthieu.Moy@grenoble-inp•fr>,
Felipe Contreras <felipe.contreras@gmail•com>,
git@vger•kernel.org, Antoine Pelisse <apelisse@gmail•com>
Subject: Re: Re* [PATCH 1/8] remote-bzr: fix export of utf-8 authors
Date: Sun, 01 Sep 2013 06:03:22 +0200 [thread overview]
Message-ID: <5222BC8A.3020605@alum.mit.edu> (raw)
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/
next prev parent reply other threads:[~2013-09-01 4:10 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-28 19:23 [PATCH 0/8] remote-{hg,bzr}: updates Felipe Contreras
2013-08-28 19:23 ` [PATCH 1/8] remote-bzr: fix export of utf-8 authors Felipe Contreras
2013-08-28 20:05 ` Matthieu Moy
2013-08-28 20:48 ` Felipe Contreras
2013-08-28 20:54 ` Antoine Pelisse
2013-08-28 21:21 ` Felipe Contreras
2013-08-28 21:38 ` Felipe Contreras
2013-08-28 21:05 ` Matthieu Moy
2013-08-28 21:21 ` Felipe Contreras
2013-08-28 21:58 ` Junio C Hamano
2013-08-28 22:25 ` Felipe Contreras
2013-08-29 21:24 ` Re* " Junio C Hamano
2013-08-29 21:25 ` Junio C Hamano
2013-09-01 4:09 ` Michael Haggerty
2013-08-29 21:28 ` Antoine Pelisse
2013-08-29 21:42 ` Junio C Hamano
2013-08-29 21:44 ` Junio C Hamano
2013-09-01 4:03 ` Michael Haggerty [this message]
2013-08-28 20:35 ` Eric Sunshine
2013-08-28 21:14 ` [PATCH v2] " Felipe Contreras
2013-08-28 19:23 ` [PATCH 2/8] remote-bzr: make bzr branches configurable per-repo Felipe Contreras
2013-08-28 19:23 ` [PATCH 3/8] remote-hg: fix test Felipe Contreras
2013-08-28 19:23 ` [PATCH 4/8] remote-hg: add missing &&s in the test Felipe Contreras
2013-08-28 19:23 ` [PATCH 5/8] remote-hg: improve basic test Felipe Contreras
2013-08-28 19:23 ` [PATCH 6/8] remote-helpers: trivial style fixes Felipe Contreras
2013-08-28 19:23 ` [PATCH 7/8] remote-helpers: cleanup more global variables Felipe Contreras
2013-08-28 19:23 ` [PATCH 8/8] remote-hg: support for notes Felipe Contreras
2013-08-29 18:45 ` Junio C Hamano
2013-08-29 21:50 ` [PATCH] " Felipe Contreras
2013-08-29 22:12 ` Junio C Hamano
2013-08-29 22:29 ` [PATCH v3] " Felipe Contreras
2013-08-29 22:49 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5222BC8A.3020605@alum.mit.edu \
--to=mhagger@alum$(echo .)mit.edu \
--cc=Matthieu.Moy@grenoble-inp$(echo .)fr \
--cc=apelisse@gmail$(echo .)com \
--cc=felipe.contreras@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox