git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Elia Pinto <gitter.spiros@gmail.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>,
	Matthieu Moy <matthieu.moy@grenoble-inp.fr>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: Patch Series v3 for "use the $( ... ) construct for command substitution"
Date: Fri, 4 Apr 2014 10:29:46 -0700	[thread overview]
Message-ID: <20140404172946.GI6851@google.com> (raw)
In-Reply-To: <CA+EOSBkF+hLOab0oJH7HuUb_KKn+1sgV_Lshwupgj-_yAAfmFw@mail.gmail.com>

Hi,

Elia Pinto wrote:

> This patch series contain the
>
>  use the $( ... ) construct for command substitution
>
> patches not already merged in ep/shell-command-substitution
> in the mantainer repository.

Thanks for working on this.  The $() form is less error-prone
than ``, so in that sense it can be worthwhile.

[...]
> Elia Pinto (140):

I admit I'm not excited to review these at all.

I wonder if it's possible to make the series easier to review.  For
example:

 * patch 0 makes preparatory changes to line wrapping or to avoid
   using `` some places to make an automatic transformation easier
 * patch 1 introduces a script to transform `` expressions to $()
   expressions
 * patch 2 just runs that script

If the script is "obviously correct" enough then there is no need
to manually go through 140 files after that point.

If the only way to get this done is to actually manually review those
140 files, I just don't think it's worth it.  The `` construct is not
*that* bad.  Another possible direction could be to add a tool to make
sure git doesn't get any new uses of ``, to let the changes flow in at
a manageable rate without too many cases of "one step forward, one
step back".

Hope that helps,
Jonathan

  reply	other threads:[~2014-04-04 17:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-04 16:52 Patch Series v3 for "use the $( ... ) construct for command substitution" Elia Pinto
2014-04-04 17:29 ` Jonathan Nieder [this message]
2014-04-04 17:40   ` Matthieu Moy
2014-04-04 18:12     ` Jonathan Nieder
2014-04-04 19:27       ` 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=20140404172946.GI6851@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=gitter.spiros@gmail.com \
    --cc=matthieu.moy@grenoble-inp.fr \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).