mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Taylor Blau <>
To: David Caro <>
Subject: Re: Skipping adding Signed-off-by even if it's not the last on git commit
Date: Tue, 6 Dec 2022 20:50:00 -0500	[thread overview]
Message-ID: <Y4/xSObs9QXvE+xR@nand.local> (raw)
In-Reply-To: <20221206170646.6lnpr6h7oprziy5b@vulcanus>

Hi David,

On Tue, Dec 06, 2022 at 06:06:46PM +0100, David Caro wrote:
> I have noticed that when requesting git commit to add the
> Signed-off-by header, it will add it even if it already exists as long
> as it's not the last in the footer.
> It seems that the functionality to do so has been there for a while
> (see [1]) and it's being used effectively on format-patch, but it was
> never enabled for the rest of commands.

Right; bab4d1097c (sequencer.c: teach append_signoff how to detect
duplicate s-o-b, 2013-02-12) introduced the APPEND_SIGNOFF_DEDUP flag,
which instructs append_signoff() to avoid adding a Signed-off-by
trailer if one already exists anywhere in the trailers section of the
target commit.

As bab4d1097c hints, this is desirable in some situations. However,
adding duplicated S-o-b trailers is useful in other situations. For
example, if you and I exchange a patch back and forth on this list, it
might go something like:

  1. I write up an initial patch, add my Singed-off-by, and send it to
     the list.

  2. You notice that some aspects can be improved, so you apply and
     modify the commit locally, adding your own Signed-off-by.

  3. I pick up your changes as part of a new version of the patch
     series, and resubmit it to the list, adding my own Signed-off-by

There are a couple of small things worth noting there. First, in (2),
the changes that you introduced were large enough to be worth crediting
(e.g., you didn't suggest a typofix or to modify the amount of spacing
on a line or similar), but not large enough to change the author of the

Likewise, in (3), the second instance of my Signed-off-by indicates that
I am OK with the changes that you wrote, and I certify the new version
of the patch as my own work.

One such instance is in 0ca6ead81e (alias.c: reject too-long cmdline
strings in split_cmdline(), 2022-09-28). Another is in b3c5f5cb04
(submodule: move core cmd_update() logic to C, 2022-03-15).

If you're curious to dig up more in your own project, here's a little
shell snippet I wrote up to find some examples:

    git rev-list --no-merges HEAD |
    while read rev
      git -P show -s -1 --format='%(trailers:key=Signed-off-by,unfold=true)' "$rev" |
      grep -v '^$' | sort | uniq -c | grep -vq "^      1 " && echo "$rev"

> I propose enabling it for commit, merge and am.

So I think that there are some legitimate uses outside of 'format-patch'
that we may want to keep the existing behavior. So I don't think we
should necessarily change the default to have other commands outside of
'format-patch' start passing APPEND_SIGNOFF_DEDUP.

But I could see a future where we add a new option that controls whether
or not we pass that flag, perhaps:

    $ git commit --signoff[=[no-]dedup]



  reply	other threads:[~2022-12-07  1:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-06 17:06 Skipping adding Signed-off-by even if it's not the last on git commit David Caro
2022-12-07  1:50 ` Taylor Blau [this message]
2022-12-07  4:11   ` Junio C Hamano
2022-12-07  8:40     ` David Caro
2022-12-07 22:13       ` Taylor Blau
2022-12-07 23:04         ` Junio C Hamano
2022-12-08  7:27           ` Jeff King
2022-12-09  1:23             ` Junio C Hamano
2022-12-09  1:42               ` Jeff King
2022-12-09  4:03                 ` Junio C Hamano
2022-12-09 20:39                   ` Jeff King
2022-12-07  4:31   ` Ævar Arnfjörð Bjarmason
2022-12-07  4:57     ` Junio C Hamano
2022-12-07 15:00     ` Konstantin Ryabitsev

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:

  List information:

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

  git send-email \
    --in-reply-to=Y4/xSObs9QXvE+xR@nand.local \ \ \ \

* 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

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).