[0/5] Unify trailer formatting functions

Message ID	pull.1694.git.1710485706.gitgitgadget@gmail.com (mailing list archive)
Headers	show Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 109CA1A27E for <git@vger.kernel.org>; Fri, 15 Mar 2024 06:55:09 +0000 (UTC) Message-ID: <pull.1694.git.1710485706.gitgitgadget@gmail.com> From: "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> Date: Fri, 15 Mar 2024 06:55:00 +0000 Subject: [PATCH 0/5] Unify trailer formatting functions Fcc: Sent Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk MIME-Version: 1.0 To: git@vger.kernel.org Cc: "Christian Couder [ ]" <chriscool@tuxfamily.org>, "Junio C Hamano [ ]" <gitster@pobox.com>, "Emily Shaffer [ ]" <nasamuffin@google.com>, "Josh Steadmon [ ]" <steadmon@google.com>, "Randall S. Becker [ ]" <rsbecker@nexbridge.com>, "Christian Couder [ ]" <christian.couder@gmail.com>, Kristoffer Haugsbakk <[code@khaugsbakk.name]>, Linus Arver <linusa@google.com>
Series	Unify trailer formatting functions \| expand [0/5] Unify trailer formatting functions [1/5] format_trailer_info(): use trailer_item objects [2/5] format_trailer_info(): drop redundant unfold_value() [3/5] format_trailer_info(): append newline for non-trailer lines [4/5] trailer: begin formatting unification [5/5] trailer: finish formatting unification

Philippe Blain via GitGitGadget March 15, 2024, 6:55 a.m. UTC

This series is based on the initial series [1], notably the v4 version of
patches 10-16 as suggested by Christian [2]. This version addresses the
review comments for those patches, namely the avoidance of (temporary) test
breakages.

The central idea is to unify the functions format_trailer_info() and
format_trailers() together (preferring the name of the latter), so that both
the interpret-trailers built-in and format_trailers_from_commit() can call
the same format_trailers() function, while maintaining feature parity.

[1]
https://lore.kernel.org/git/pull.1632.v4.git.1707196348.gitgitgadget@gmail.com/
[2]
https://lore.kernel.org/git/CAP8UFD08F0V13X0+CJ1uhMPzPWVMs2okGVMJch0DkQg5M3BWLA@mail.gmail.com/

Linus Arver (5):
  format_trailer_info(): use trailer_item objects
  format_trailer_info(): drop redundant unfold_value()
  format_trailer_info(): append newline for non-trailer lines
  trailer: begin formatting unification
  trailer: finish formatting unification

 trailer.c | 81 ++++++++++++++++++++-----------------------------------
 trailer.h | 13 +++------
 2 files changed, 32 insertions(+), 62 deletions(-)


base-commit: 4f9b731bdeccffa1b13e5edf4bc0428b8d49704e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1694%2Flistx%2Ftrailer-api-part-2-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1694/listx/trailer-api-part-2-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1694

Junio C Hamano March 15, 2024, 5:20 p.m. UTC | #1

Not about the series, but about the way it was sent.

The messages in this series have exactly the same kind of breakages
in the recipient names/addresses we recently saw:

    https://lore.kernel.org/git/xmqqh6hkxox6.fsf@gitster.g/

Human-readable names with a SP inside [square bracket] pair
appended, and one of the addresses had that square bracket applied
inside <angle bracket> pair and breaking MTAs (I manually fixed
khaugsbakk's address before sending this response, so replying to
this messages should be OK).

What are you and Aryan's pull.1675.v3 did differently from other
series sent via GGG to trigger this, I have to wonder?  Without
knowing it, it would be hard to avoid seeing these broken addresses
again.

Thanks.

Kristoffer Haugsbakk March 15, 2024, 6:26 p.m. UTC | #2

On Fri, Mar 15, 2024, at 18:20, Junio C Hamano wrote:
> Not about the series, but about the way it was sent.
>
> The messages in this series have exactly the same kind of breakages
> in the recipient names/addresses we recently saw:
>
>     https://lore.kernel.org/git/xmqqh6hkxox6.fsf@gitster.g/
>
> Human-readable names with a SP inside [square bracket] pair
> appended, and one of the addresses had that square bracket applied
> inside <angle bracket> pair and breaking MTAs (I manually fixed
> khaugsbakk's address before sending this response, so replying to
> this messages should be OK).
>
> What are you and Aryan's pull.1675.v3 did differently from other
> series sent via GGG to trigger this, I have to wonder?  Without
> knowing it, it would be hard to avoid seeing these broken addresses
> again.
>
> Thanks.

I’m the only common CC on these two. Maybe dropping me for the possible
next round would fix it. Would be interesting to see.

Cheers

Junio C Hamano March 15, 2024, 7:10 p.m. UTC | #3

"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:

> I’m the only common CC on these two. Maybe dropping me for the possible
> next round would fix it. Would be interesting to see.

But there is nothing strange in your name or address.  There may be
"something" that confuses GGG about how they spelled your name and
address, and if the next user does that same "something" for some
other recipient, it wouldn't be you but that other recipient with
the same issue, I am afraid.

Linus Arver March 15, 2024, 9:36 p.m. UTC | #4

Junio C Hamano <gitster@pobox.com> writes:

> Not about the series, but about the way it was sent.
>
> The messages in this series have exactly the same kind of breakages
> in the recipient names/addresses we recently saw:
>
>     https://lore.kernel.org/git/xmqqh6hkxox6.fsf@gitster.g/
>
> Human-readable names with a SP inside [square bracket] pair
> appended, and one of the addresses had that square bracket applied
> inside <angle bracket> pair and breaking MTAs (I manually fixed
> khaugsbakk's address before sending this response, so replying to
> this messages should be OK).

UGH, I'm so sorry about that.

> What are you and Aryan's pull.1675.v3 did differently from other
> series sent via GGG to trigger this, I have to wonder?

I realize now that it's because I copy/pasted the "Cc: ..." lines in the PR
description from
https://github.com/gitgitgadget/git/pull/1632#issue-2068188239, such
that when I pasted those in for the PR description for this series at
https://github.com/gitgitgadget/git/pull/1694#issue-2187804953, it
carried over the email addresses as Markdown-formatted hyperlinks.
Currently it reads

    Cc: Christian Couder [chriscool@tuxfamily.org](mailto:chriscool@tuxfamily.org)
    Cc: Junio C Hamano [gitster@pobox.com](mailto:gitster@pobox.com)
    Cc: Emily Shaffer [nasamuffin@google.com](mailto:nasamuffin@google.com)
    cc: Josh Steadmon [steadmon@google.com](mailto:steadmon@google.com)
    cc: Randall S. Becker [rsbecker@nexbridge.com](mailto:rsbecker@nexbridge.com)
    cc: Christian Couder [christian.couder@gmail.com](mailto:christian.couder@gmail.com)
    cc: "Kristoffer Haugsbakk" [code@khaugsbakk.name](mailto:code@khaugsbakk.name)
    cc: "Kristoffer Haugsbakk" <code@khaugsbakk.name>

when I click on "edit", where the last line must be from your manual fix
which GGG picked up. I've cleaned up the PR description manually now,
and for this message I'm also attempting to clean up those square
brackets.

Junio C Hamano March 15, 2024, 9:43 p.m. UTC | #5

Linus Arver <linusa@google.com> writes:

> I realize now that it's because I copy/pasted the "Cc: ..." lines in the PR
> description from
> https://github.com/gitgitgadget/git/pull/1632#issue-2068188239, such
> that when I pasted those in for the PR description for this series at
> https://github.com/gitgitgadget/git/pull/1694#issue-2187804953, it
> carried over the email addresses as Markdown-formatted hyperlinks.
> Currently it reads
>
>     Cc: Christian Couder [chriscool@tuxfamily.org](mailto:chriscool@tuxfamily.org)
>     Cc: Junio C Hamano [gitster@pobox.com](mailto:gitster@pobox.com)
>     Cc: Emily Shaffer [nasamuffin@google.com](mailto:nasamuffin@google.com)
>     cc: Josh Steadmon [steadmon@google.com](mailto:steadmon@google.com)
>     cc: Randall S. Becker [rsbecker@nexbridge.com](mailto:rsbecker@nexbridge.com)
>     cc: Christian Couder [christian.couder@gmail.com](mailto:christian.couder@gmail.com)
>     cc: "Kristoffer Haugsbakk" [code@khaugsbakk.name](mailto:code@khaugsbakk.name)
>     cc: "Kristoffer Haugsbakk" <code@khaugsbakk.name>
>
> when I click on "edit", where the last line must be from your manual fix
> which GGG picked up. I've cleaned up the PR description manually now,
> and for this message I'm also attempting to clean up those square
> brackets.

The last time I asked the author with the same question, we
unfortunately did not get a clear answer on what should be avoided
(probably the author was happy enough with the message resulted from
the updated pull request and forgot that it is more important to
help others from having the same issue).  Now we have an example we
can point at when a similar issue arises.

Thanks.

Junio C Hamano March 26, 2024, 9:57 p.m. UTC | #6

"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This series is based on the initial series [1], notably the v4 version of
> patches 10-16 as suggested by Christian [2]. This version addresses the
> review comments for those patches, namely the avoidance of (temporary) test
> breakages.

It has been 10 days but we haven't seen any reviews on this one.  It
could be that it did not get to intended mailboxes due to the header
corruption (I manually fixed in this message), but without reviews
we cannot move forward.

Linus Arver April 2, 2024, 12:27 a.m. UTC | #7

Junio C Hamano <gitster@pobox.com> writes:

> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> This series is based on the initial series [1], notably the v4 version of
>> patches 10-16 as suggested by Christian [2]. This version addresses the
>> review comments for those patches, namely the avoidance of (temporary) test
>> breakages.
>
> It has been 10 days but we haven't seen any reviews on this one.  It
> could be that it did not get to intended mailboxes due to the header
> corruption (I manually fixed in this message), but without reviews
> we cannot move forward.

Adding a couple more folks to the CC list according to the
contrib/contacts/git-contacts script. Thanks!

Junio C Hamano April 15, 2024, 9:02 p.m. UTC | #8

Linus Arver <linusa@google.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>>> This series is based on the initial series [1], notably the v4 version of
>>> patches 10-16 as suggested by Christian [2]. This version addresses the
>>> review comments for those patches, namely the avoidance of (temporary) test
>>> breakages.
>>
>> It has been 10 days but we haven't seen any reviews on this one.  It
>> could be that it did not get to intended mailboxes due to the header
>> corruption (I manually fixed in this message), but without reviews
>> we cannot move forward.
>
> Adding a couple more folks to the CC list according to the
> contrib/contacts/git-contacts script. Thanks!

As we saw no responses after two weeks, I took some time to scan
these patches myself for the final time.  I didn't see anything
gravely wrong in it and am tempted to merge it down to 'next'.

Unless I hear objections in the next few days, that is.

Thanks.

Johannes Schindelin July 2, 2024, 9:37 a.m. UTC | #9

Hi,

On Fri, 15 Mar 2024, Linus Arver wrote:

> Junio C Hamano <gitster@pobox.com> writes:
>
> > Not about the series, but about the way it was sent.
> >
> > The messages in this series have exactly the same kind of breakages
> > in the recipient names/addresses we recently saw:
> >
> >     https://lore.kernel.org/git/xmqqh6hkxox6.fsf@gitster.g/
> >
> > Human-readable names with a SP inside [square bracket] pair
> > appended, and one of the addresses had that square bracket applied
> > inside <angle bracket> pair and breaking MTAs (I manually fixed
> > khaugsbakk's address before sending this response, so replying to
> > this messages should be OK).
>
> UGH, I'm so sorry about that.
>
> > What are you and Aryan's pull.1675.v3 did differently from other
> > series sent via GGG to trigger this, I have to wonder?
>
> I realize now that it's because I copy/pasted the "Cc: ..." lines in the PR
> description from
> https://github.com/gitgitgadget/git/pull/1632#issue-2068188239, such
> that when I pasted those in for the PR description for this series at
> https://github.com/gitgitgadget/git/pull/1694#issue-2187804953, it
> carried over the email addresses as Markdown-formatted hyperlinks.
> Currently it reads
>
>     Cc: Christian Couder [chriscool@tuxfamily.org](mailto:chriscool@tuxfamily.org)
>     Cc: Junio C Hamano [gitster@pobox.com](mailto:gitster@pobox.com)
>     Cc: Emily Shaffer [nasamuffin@google.com](mailto:nasamuffin@google.com)
>     cc: Josh Steadmon [steadmon@google.com](mailto:steadmon@google.com)
>     cc: Randall S. Becker [rsbecker@nexbridge.com](mailto:rsbecker@nexbridge.com)
>     cc: Christian Couder [christian.couder@gmail.com](mailto:christian.couder@gmail.com)
>     cc: "Kristoffer Haugsbakk" [code@khaugsbakk.name](mailto:code@khaugsbakk.name)
>     cc: "Kristoffer Haugsbakk" <code@khaugsbakk.name>
>
> when I click on "edit", where the last line must be from your manual fix
> which GGG picked up. I've cleaned up the PR description manually now,
> and for this message I'm also attempting to clean up those square
> brackets.

I would love to let myself be nerdsniped into working on this, alas,
I cannot afford that before I learn the trick to stretch time.

So I did the next-best thing and jotted down pointers for any volunteer
who wants to work on this:
https://github.com/gitgitgadget/gitgitgadget/issues/1645#issuecomment-2202545542

Ciao,
Johannes

[0/5] Unify trailer formatting functions

Message

Comments