git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: Elijah Newren <newren@gmail.com>,
	 git@vger.kernel.org,  Patrick Steinhardt <ps@pks.im>,
	 Jeff King <peff@peff.net>,
	 "brian m . carlson" <sandals@crustytoothpaste.net>,
	 Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	 Christian Couder <chriscool@tuxfamily.org>
Subject: Re: [PATCH v4] fast-(import|export): improve on commit signature output format
Date: Fri, 20 Jun 2025 12:20:38 -0700	[thread overview]
Message-ID: <xmqqfrfu2pa1.fsf@gitster.g> (raw)
In-Reply-To: <CAP8UFD24LayEi-=PREqbiyC7PeJbYEP1ghOZ+T-ma=DZVaPtSg@mail.gmail.com> (Christian Couder's message of "Fri, 20 Jun 2025 18:12:36 +0200")

Christian Couder <christian.couder@gmail.com> writes:

tldr; 2 brief requests.  Please

 - Be gentle to people and expect that it is normal for them to be
   off of the list for a few weeks (or even more), not able to give
   a timely comments;

 - Fully stand behind your own patch (unless it is an RFC), even if
   some of the idea came from elsewhere.

>> Why would it need to say what type of signature it is?  Don't the
>> ascii armor lines have e.g. "----BEGIN PGP SIGNATURE----" and "----END
>> PGP SIGNATURE----" around it, which fast-import can read as well as
>> fast-export?  Is the idea that we strip those lines and now need to
>> replace the information we lost?
>
> In https://lore.kernel.org/git/aAq1nvcPRlIPal5l@tapette.crustytoothpaste.net/
> brian said:
>
> "These should be separate fields: one for the hash algorithm and one for
> the protocol.  Alternatively, we can just keep the hash algorithm field
> and parse the protocol by reading the first line, which will differ for
> different protocols."
>
> It would have been nice if you had then said that you prefer not to
> have the protocol.

Let's remember to be gentle for those who give varluable feedbacks
but may not be always on this list.  A late comment on a topic that
has not hit 'next' is much better than a late comment after the
topic hits 'next', or no comment at all.

Also, even if the idea came from somebody else, if you agreed to the
idea and made it part of your submission, then it would be better to
explain it in your own words, in the most appropriate way to answer
the question asked (e.g. the original from Brian and the question by
Elijah may have stress on different aspect of the problem).

> My opinion was that it was better for tools processing fast-export
> output to have the protocol as they have to parse the "gppsig ..."
> line anyway. So it should be easier for them than to parse the ascii
> armor lines.

And if you do not agree with Brian's, perhaps discuss a bit more to
(1) either convince yourself that Brian's idea is better and rewrite
your code to adopt the idea, or (2) explain the reason why your "the
importer reads and parses anyway" is better design and stick to it.

> ...
> Yeah, I would have been happy if we could have been aligned with the
> goals of the format and the fields earlier, but better late than
> never.
>
>> Thanks for working on this topic.
>
> Thanks for reviewing.

Thanks.

  reply	other threads:[~2025-06-20 19:20 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-24 20:39 [PATCH] fast-(import|export): improve on the signature algorithm name Christian Couder
2025-04-24 21:19 ` Junio C Hamano
2025-04-24 21:59   ` Elijah Newren
2025-04-24 22:58     ` Junio C Hamano
2025-05-26 10:35       ` Christian Couder
2025-05-27 15:18         ` Junio C Hamano
2025-05-28 17:29           ` Junio C Hamano
2025-05-28 20:06             ` Elijah Newren
2025-05-28 21:59               ` Junio C Hamano
2025-05-28 23:15                 ` Elijah Newren
2025-05-29  3:14                   ` Junio C Hamano
2025-06-02 15:56                     ` Christian Couder
2025-06-02 15:56             ` Christian Couder
2025-06-02 16:20               ` Junio C Hamano
2025-05-26 10:34   ` Christian Couder
2025-04-24 21:41 ` Elijah Newren
2025-05-26 10:34   ` Christian Couder
2025-04-24 22:05 ` brian m. carlson
2025-05-26 10:35   ` Christian Couder
2025-04-24 23:25 ` Junio C Hamano
2025-05-26 10:33 ` [PATCH v2 0/6] extract algo information from signatures Christian Couder
2025-05-26 10:33   ` [PATCH v2 1/6] gpg-interface: simplify ssh fingerprint parsing Christian Couder
2025-05-26 10:33   ` [PATCH v2 2/6] gpg-interface: use left shift to define GPG_VERIFY_* Christian Couder
2025-05-26 10:33   ` [PATCH v2 3/6] doc/verify-commit: update and improve the whole doc Christian Couder
2025-05-26 10:33   ` [PATCH v2 4/6] gpg-interface: extract hash algorithm from signature status output Christian Couder
2025-05-26 10:33   ` [PATCH v2 5/6] gpg-interface: extract SSH key type " Christian Couder
2025-05-26 10:33   ` [PATCH v2 6/6] verify-commit: add a --summary flag Christian Couder
2025-05-26 16:03   ` [PATCH v2 0/6] extract algo information from signatures Elijah Newren
2025-06-19 13:38     ` Christian Couder
2025-06-02 22:17   ` brian m. carlson
2025-06-19 13:37     ` Christian Couder
2025-06-18 15:18   ` [PATCH v3] fast-(import|export): improve on commit signature output format Christian Couder
2025-06-19 13:36     ` [PATCH v4] " Christian Couder
2025-06-19 14:55       ` Junio C Hamano
2025-07-08  9:16         ` Christian Couder
2025-06-19 21:44       ` Elijah Newren
2025-06-20 16:12         ` Christian Couder
2025-06-20 19:20           ` Junio C Hamano [this message]
2025-07-08  9:16             ` Christian Couder
2025-06-26 19:11           ` Elijah Newren
2025-07-08  9:16             ` Christian Couder
2025-07-07 22:58       ` Junio C Hamano
2025-07-08  3:35         ` Christian Couder
2025-07-08  5:03           ` Junio C Hamano
2025-07-08  6:38             ` Patrick Steinhardt
2025-07-08 11:08               ` Christian Couder
2025-07-08 16:38                 ` Junio C Hamano
2025-07-09  0:19                   ` Christian Couder
2025-07-09 15:35                     ` Junio C Hamano
2025-07-10  8:25                     ` Patrick Steinhardt
2025-07-10 15:29                       ` Christian Couder
2025-07-10 15:33                       ` Junio C Hamano
2025-07-08 10:17             ` Christian Couder
2025-07-08  9:17       ` [PATCH v5] " Christian Couder
2025-07-08 21:58         ` Junio C Hamano
2025-07-08 23:08         ` Elijah Newren
2025-07-09  0:03           ` Junio C Hamano
2025-07-09  0:10             ` Elijah Newren
2025-07-09 10:18             ` Christian Couder
2025-07-09 10:15           ` Christian Couder
2025-07-09 14:12         ` [PATCH v6] " Christian Couder
2025-07-09 23:14           ` Junio C Hamano
2025-07-14 21:07           ` Elijah Newren
2025-07-14 21:23             ` Junio C Hamano
2025-07-25 16:11               ` Christian Couder

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=xmqqfrfu2pa1.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    --cc=sandals@crustytoothpaste.net \
    /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).