git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Mike Hommey <mh@glandium.org>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] gpg-interface: Add some output from gpg when it errors out.
Date: Wed, 25 Jan 2017 18:37:55 -0800	[thread overview]
Message-ID: <xmqq8tpy7dh8.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170125235410.byxwmo7o7zdszzot@glandium.org> (Mike Hommey's message of "Thu, 26 Jan 2017 08:54:10 +0900")

Mike Hommey <mh@glandium.org> writes:

> On Wed, Jan 25, 2017 at 03:04:38PM -0800, Junio C Hamano wrote:
> ...
>> Overall I think this is a good thing to do.  Instead of eating the
>> status output, showing what we got, especially when we know the
>> command failed, would make the bug-hunting of user's environment
>> easier, like you showed above.
>> 
>> The only thing in the design that makes me wonder is the filtering
>> out based on "[GNUPG:]" prefix.  Why do we need to filter them out?
>
> The [GNUPG:] lines are part of the status-fd protocol. They show details
> that don't really seem interesting to the user. In fact, they even
> contain the signed message (yes, in my case, it turns out gpg was
> actually still signing, but returned an error code...).

OK, that detail was what was missing in the proposed log message.
Without that "why do we filter?", readers cannot correctly assess
why it is a good idea.  I wasn't arguing against filtering it; I
just wanted to make sure "git log" readers (and "git show" user
after "git blame" identifies this change in the history) will know
how we decided to filter lines with the prefix.  

With that information recorded in the log (or in-code comment, or
both), if it turns out that some lines with the prefix are useful
(or some other lines without the prefix are not very useful), they
can tweak the filtering criteria as appropriate, with confidence
that they _know_ for what purpose the initial "filter lines with the
prefix" was trying to serve, and their update is still in the same
spirit as the original, only executed better.

Thanks.

  reply	other threads:[~2017-01-26  2:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-25  3:04 [PATCH] gpg-interface: Add some output from gpg when it errors out Mike Hommey
2017-01-25 23:04 ` Junio C Hamano
2017-01-25 23:54   ` Mike Hommey
2017-01-26  2:37     ` Junio C Hamano [this message]
2017-01-26  2:55       ` Mike Hommey
2017-01-26 18:29         ` Junio C Hamano
2017-01-26  5:21     ` 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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq8tpy7dh8.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=mh@glandium.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).