git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Santiago Torres <santiago@nyu.edu>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Jan Palus <jan.palus@gmail.com>
Subject: Re: [PATCH 3/3] t7004, t7030: fix here-doc syntax errors
Date: Wed, 22 Mar 2017 18:41:24 -0400	[thread overview]
Message-ID: <20170322224124.u3eax4ui3y4saxks@sigill.intra.peff.net> (raw)
In-Reply-To: <20170322223441.w32y464jqbnxnzna@LykOS.localdomain>

On Wed, Mar 22, 2017 at 06:34:43PM -0400, Santiago Torres wrote:

> > I worked up the patch to do that (see below), but I got stumped trying
> > to write the commit message. Perhaps that is what the test intended, but
> > I don't think tag's --format understands "%G" codes at all. So you
> > cannot tell from the output if a tag was valid or not; you have to check
> > the exit code separately.
> > 
> > And if you do something like:
> > 
> >   git tag -v --format='%(tag)' foo bar |
> >   while read tag
> >   do
> >      ...
> >   done
> > 
> > you cannot tell at all which ones are bogus. Whereas with the current
> > behavior, the bogus ones are quietly omitted. Which can also be
> > confusing, but I'd think would generally err on the side of caution.
> 
> In that case, something like this would be closer to the desired
> behavior?

Yes, though you can spell:

  cat >expect <<-\EOF
  EOF

as just:

  >expect

> I'm also unsure on what would be the right thing to put on the commit
> message.

I think the argument is:

  1. It's safer not to expound on tags that have failed verification (so
     that the caller cannot accidentally use them). Especially since the
     --format cannot tell anything about the GPG status.

     That means that

       tag=$(git verify-tag --format='%(tag)' foo)

     can use a non-blank $tag without having to wonder whether it is
     valid or not.

and

  2. That's what we've done since the feature was released.

The only thing that would give me pause is if were to later add
%G-like formatters, and then:

  xargs git verify-tag --format='%(gpg:status) %(tag)' |
  while read status tag
  do
     ...
  done

would become useful, but we'd be tied to the behavior that we omit the
tag when the gpg verification failed (for backwards compatibility).
OTOH, we could perhaps make the rule "ignored unless %(gpg) formatters
are used". Which would be backwards-compatible and safe for old formats,
and work correctly for new ones.

-Peff

  reply	other threads:[~2017-03-22 22:41 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-22 17:35 EOF test fixes (t5615/t7004) Jan Palus
2017-03-22 18:28 ` EOF test fixes (t7030/t7406) Jan Palus
2017-03-22 18:47 ` EOF test fixes (t5615/t7004) Stefan Beller
2017-03-22 19:43 ` Junio C Hamano
2017-03-22 20:08 ` [PATCH 0/3] fix "here-doc" syntax errors Junio C Hamano
2017-03-22 20:08   ` [PATCH 1/3] t5615: fix a here-doc syntax error Junio C Hamano
2017-03-22 21:02     ` Jeff King
2017-03-22 20:08   ` [PATCH 2/3] t7406: fix here-doc syntax errors Junio C Hamano
2017-03-22 21:07     ` Jeff King
2017-03-22 21:32       ` Stefan Beller
2017-03-22 21:39         ` Jeff King
2017-03-22 21:49           ` [PATCH] t7406: correct test case for submodule-update initial population Stefan Beller
2017-03-22 21:59             ` Jeff King
2017-03-22 22:07               ` Stefan Beller
2017-03-22 22:09                 ` Jeff King
2017-03-22 22:14                 ` Jonathan Nieder
2017-03-22 22:12               ` Junio C Hamano
2017-03-22 22:24                 ` Jeff King
2017-03-22 22:28                   ` Stefan Beller
2017-03-22 21:34       ` [PATCH 2/3] t7406: fix here-doc syntax errors Junio C Hamano
2017-03-22 20:08   ` [PATCH 3/3] t7004, t7030: " Junio C Hamano
2017-03-22 21:10     ` Jeff King
2017-03-22 21:43       ` Santiago Torres
2017-03-22 22:04         ` Jeff King
2017-03-22 22:04       ` Junio C Hamano
2017-03-22 22:15         ` Santiago Torres
2017-03-22 22:22           ` Jeff King
2017-03-22 22:34             ` Santiago Torres
2017-03-22 22:41               ` Jeff King [this message]
2017-03-22 22:47                 ` Junio C Hamano
2017-03-22 22:51                 ` Santiago Torres
2017-03-23 22:00                   ` Junio C Hamano
2017-03-23 22:28                     ` Santiago Torres
2017-03-23 23:49                     ` Jeff King
2017-03-24 16:45                       ` Junio C Hamano
2017-03-24 16:49                         ` Jeff King
2017-03-24 18:00                           ` Jeff King
2017-03-24 18:04                           ` Junio C Hamano
2017-03-24 18:16                             ` Jeff King
2017-03-22 22:38             ` Junio C Hamano
2017-03-22 22:40   ` [PATCH 0/3] fix "here-doc" " Jan Palus
2017-03-23  2:12   ` [PATCH] tests: lint for run-away here-doc Junio C Hamano
2017-03-23  5:43     ` [PATCH v2] " Junio C Hamano
2017-03-24  1:29       ` Jonathan Nieder
2017-03-24  2:45         ` Junio C Hamano
2017-03-24  3:59       ` Jeff King

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=20170322224124.u3eax4ui3y4saxks@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jan.palus@gmail.com \
    --cc=santiago@nyu.edu \
    /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).