From: Jeff King <peff@peff.net>
To: Jacob Keller <jacob.keller@gmail.com>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
Jacob Keller <jacob.e.keller@intel.com>,
Git List <git@vger.kernel.org>,
Mark Strapetz <marc.strapetz@syntevo.com>,
Stefan Beller <sbeller@google.com>,
Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v2] git: submodule honor -c credential.* from command line
Date: Thu, 25 Feb 2016 01:23:11 -0500 [thread overview]
Message-ID: <20160225062311.GA5541@sigill.intra.peff.net> (raw)
In-Reply-To: <CA+P7+xpkbZyuQ7gpC+_KzJPfGo+xHpVFWEmr1oS7FD30jjJSOQ@mail.gmail.com>
On Wed, Feb 24, 2016 at 10:19:05PM -0800, Jacob Keller wrote:
> >> + /* combined all the values before we quote them */
> >
> > Comment repeats what the code already says, thus not terribly useful.
> >
> > Also: s/combined/combine/
> >
> I tend to make comments like this when I change the obvious way it was
> done, I commented this because I based it on a scratch patch from Jeff
> that didn't have them put together before quoting.
That's a good point. The _what_ is not interesting here, but the _why_
might be. Namely that we must quote the whole thing as a unit, or the
parser on the receiving end will not be able to read it.
I'd also be amenable to relaxing the parser (which is as strict as it is
only out of laziness, and the fact that it was reading the output only
of its nearby generator function). But I can understand if you don't
feel like digging into that.
-Peff
next prev parent reply other threads:[~2016-02-25 6:23 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-24 23:59 [PATCH v2] git: submodule honor -c credential.* from command line Jacob Keller
2016-02-25 0:27 ` Eric Sunshine
2016-02-25 1:45 ` Jeff King
2016-02-25 6:19 ` Jacob Keller
2016-02-25 6:23 ` Jeff King [this message]
2016-02-25 6:24 ` Jacob Keller
2016-02-25 1:41 ` Jeff King
2016-02-25 6:23 ` Jacob Keller
2016-02-25 7:00 ` Jeff King
2016-02-25 7:11 ` Jeff King
2016-02-25 18:07 ` Jacob Keller
2016-02-25 18:51 ` Jacob Keller
2016-02-25 20:32 ` Junio C Hamano
2016-02-25 21:48 ` Jacob Keller
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=20160225062311.GA5541@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jacob.e.keller@intel.com \
--cc=jacob.keller@gmail.com \
--cc=marc.strapetz@syntevo.com \
--cc=sbeller@google.com \
--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).