All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Danny Lin <danny0838@gmail.com>
Cc: git develop <git@vger.kernel.org>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [PATCH] contrib/subtree: portability fix for string printing
Date: Fri, 08 May 2015 10:49:38 -0700	[thread overview]
Message-ID: <xmqqy4kzklhp.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1431046619-2340-1-git-send-email-danny0838@gmail.com> (Danny Lin's message of "Fri, 8 May 2015 08:56:59 +0800")

Danny Lin <danny0838@gmail.com> writes:

> Replace echo using printf in debug() and say() for
> better portability.
>
> Also re-wrap previous 'say -n "$str<CR>"' using a new
> function progress() to prevent CR chars included in the
> source code, which could be mal-processed in some shells.
> For example, MsysGit trims CR before executing a shell
> script file in order to make it work right on Windows
> even if it uses CRLF as linefeeds.
>
> Signed-off-by: Danny Lin <danny0838@gmail.com>
> ---

Thanks, this looks good.  Will apply with a little bit of tweak in
the log message.

Just for future reference, when shooting many iterations of the same
patch in a short timeframe, please be aware that the recipient may
not get the messages in the order you sent, and that it may not be
apparent to the recipients what changed between the iterations.
What we commonly do around here to address these issues is to
mention what changed from the previous one below the "---" line
before the diffstat.  I would have done something like this if I
were doing this patch, for example:

        ...
        even if it uses CRLF as linefeeds.

        Signed-off-by: Danny Lin <danny0838@gmail.com>
        ---

        * The previous one still used "log" helper by mistake even
          though I removed the implementation of it and decided to
          use "echo" for non-tricky cases.  This fixes it.

          contrib/subtree/git-subtree.sh | 13 ++++++++++---
        ...

  reply	other threads:[~2015-05-08 17:49 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-07  3:39 [PATCH] contrib/subtree: fix linefeeds trimming for cmd_split() Danny Lin
2015-05-07  3:43 ` Danny Lin
2015-05-07  5:10   ` Danny Lin
2015-05-07 18:33     ` Junio C Hamano
2015-05-08  0:51       ` [PATCH] contrib/subtree: portability fix for string printing Danny Lin
2015-05-08  0:56       ` Danny Lin
2015-05-08 17:49         ` Junio C Hamano [this message]
2015-05-08 17:56           ` Eric Sunshine
2015-05-08 18:44             ` Junio C Hamano
2015-05-08 18:55               ` Eric Sunshine

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=xmqqy4kzklhp.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=danny0838@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.