All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jim Hill <gjthill@gmail.com>
Cc: git@vger.kernel.org, Aaron Schrab <aaron@schrab.com>
Subject: Re: [PATCH w/signoff] pre-push.sample: Remove unwanted `IFS=' '`.
Date: Sun, 21 Dec 2014 10:50:33 -0800	[thread overview]
Message-ID: <xmqqtx0obzwm.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1419186337-20348-1-git-send-email-gjthill@gmail.com> (Jim Hill's message of "Sun, 21 Dec 2014 10:25:37 -0800")

Jim Hill <gjthill@gmail.com> writes:

> Signed-off-by: Jim Hill <gjthill@gmail.com>
> ---

Please clarify "unwanted" in the proposed commit log message.

It looks to me that the assignment very much deliberate.  We know
refnames and 40-hex object names do not contain SP, and the hook is
fed (quoting from Documentation/githooks.txt) like this:

    Information about what is to be pushed is provided on the hook's standard
    input with lines of the form:

      <local ref> SP <local sha1> SP <remote ref> SP <remote sha1> LF

so setting IFS to SP alone smells as an attempt to ensure that the
"read" in each loop iteration would split at SP and nothing else;
Aaron Schrab CC'ed who did the original in 87c86dd1 (Add sample
pre-push hook script, 2013-01-13).

Also you would notice by reading "git shortlog" of our history that
s/Remove/remove/ on the subject line would avoid this entry stand out
among others unnecessarily, but that is minor.

>  templates/hooks--pre-push.sample | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample
> index 69e3c67..6187dbf 100755
> --- a/templates/hooks--pre-push.sample
> +++ b/templates/hooks--pre-push.sample
> @@ -24,7 +24,6 @@ url="$2"
>  
>  z40=0000000000000000000000000000000000000000
>  
> -IFS=' '
>  while read local_ref local_sha remote_ref remote_sha
>  do
>  	if [ "$local_sha" = $z40 ]

  reply	other threads:[~2014-12-21 18:50 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-21 18:14 [PATCH] pre-push.sample: Remove unwanted `IFS=' '` Jim Hill
2014-12-21 18:25 ` [PATCH w/signoff] " Jim Hill
2014-12-21 18:50   ` Junio C Hamano [this message]
2014-12-21 19:12     ` Jim Hill
2014-12-21 22:49       ` Junio C Hamano
2014-12-23  2:08         ` Junio C Hamano
2014-12-21 19:26 ` [PATCH v2] pre-push.sample: remove " Jim Hill

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=xmqqtx0obzwm.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=aaron@schrab.com \
    --cc=git@vger.kernel.org \
    --cc=gjthill@gmail.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.