public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail•com>
To: "Junio C Hamano" <gitster@pobox•com>
Cc: "Derrick Stolee" <stolee@gmail•com>,
	git@vger•kernel.org,
	"Oswald Buddenhagen" <oswald.buddenhagen@gmx•de>
Subject: Re: [PATCH v3] doc: add caveat about turning off commit-graph
Date: Wed, 20 May 2026 09:10:54 +0200	[thread overview]
Message-ID: <344cc7d8-4ab3-40f4-9564-80e5888c5bc9@app.fastmail.com> (raw)
In-Reply-To: <xmqq8q9qwxrr.fsf@gitster.g>

On Mon, May 11, 2026, at 03:16, Junio C Hamano wrote:
>>>[snip]
>>
>> It’s certainly not necessary, yeah. :)
>>
>> I am basing this on a recollection of someone quoting this from
>> SubmittingPatches:
>>
>>     Do not forget to add trailers such as `Acked-by:`, `Reviewed-by:` and
>>     `Tested-by:` lines as necessary to credit people who helped your
>>     patch, and "cc:" them when sending such a final version for inclusion.
>>
>> They said that this was outdated since Junio does it himself. But then
>> Junio replied and said that it’s good/better if the contributor does it.
>
> I used to say "let me do this to skip one extra roundtrip" but I
> stopped saying so.  Perhaps I should be a bit more explicit and stop
> being silently nice to contributors who do not follow the guidelines
> to the letter in order to unconfuse you and your friends.  It
> actually is a tempting thought.

I am personally okay with adding these trailers and think it’s nice to
document such review/ack/etc. interactions.

Just considering this part in isolation, I imagine that you not filling
in missing acks etc. will lead to less such trailers because (1) most
contributors don’t seem to follow up with such updates if the only
change is the trailers section, and (2) most people here (culturally)
don’t add explicit trailer lines in their replies (e.g. an “LGTM” is
clearly an ack, but not explicit).

>>     Well, this is another instance that I may be trying to be too
>>     helpful and over extending myself, which does not make the process
>>     scale well (the other one being the "one final resend after the
>>     list reached a consensus").
>>
>>     If the authors collect Acks and Reviewed-by's and resend after the
>>     list reached the concensus, it may take one extra iteration, but I
>>     no longer have to keep track of these trailers myself, which couldOn Mon, May 11, 2026, at 03:16, Junio C Hamano wrote:
>>>[snip]
>>
>> It’s certainly not necessary, yeah. :)
>>
>> I am basing this on a recollection of someone quoting this from
>> SubmittingPatches:
>>
>>     Do not forget to add trailers such as `Acked-by:`, `Reviewed-by:` and
>>     `Tested-by:` lines as necessary to credit people who helped your
>>     patch, and "cc:" them when sending such a final version for inclusion.
>>
>> They said that this was outdated since Junio does it himself. But then
>> Junio replied and said that it’s good/better if the contributor does it.
>
> I used to say "let me do this to skip one extra roundtrip" but I
> stopped saying so.  Perhaps I should be a bit more explicit and stop
> being silently nice to contributors who do not follow the guidelines
> to the letter in order to unconfuse you and your friends.  It
> actually is a tempting thought.

I am personally okay with adding these trailers and think it’s nice to
document such review/ack/etc. interactions.

Just considering this part in isolation, I imagine that you not filling
in missing acks etc. will lead to less such trailers because (1) most
contributors don’t seem to follow up with such updates if the only
change is the trailers section, and (2) most people here (culturally)
don’t add explicit trailer lines in their replies (e.g. an “LGTM” is
clearly an ack, but not explicit).

>>     Well, this is another instance that I may be trying to be too
>>     helpful and over extending myself, which does not make the process
>>     scale well (the other one being the "one final resend after the
>>     list reached a consensus").
>>
>>     If the authors collect Acks and Reviewed-by's and resend after the
>>     list reached the concensus, it may take one extra iteration, but I
>>     no longer have to keep track of these trailers myself, which could
>>     be a big win.
>>
>>     So, I dunno.
>>
>> In conclusion for now: I dunno. :)
>
> I do not know either, but if we agree that everybody should do so
> themselves and I should refrain from applying the ones that lack
> Acks, I can adjust.  There will be lot of unapplied patches left on
> the mailing list initially until the contributors adjust their
> behaviour, but in the long run it may be beneficial?

Okay, if the proposal is to *not* e.g. graduate series to `next` that
haven’t applied the acks etc. then I understand how it will likely lead
to some stalls until people adjust.

To be clear, I imagine this is how it would play out:

• The series in itself is ready for `next` and has no unapplied acks
  etc.: it graduates to `next`
• The series in itself is ready for `next` but has unapplied acks etc.:
  it does not graduate to `next` since the contributor should send a new
  version with the trailer changes

***

There is also the paragraph previous to the trailer one:

    After the list reached a consensus that it is a good idea to apply the
    patch, re-send it with "To:" set to the maintainer{current-maintainer}
    and "cc:" the list{git-ml} for inclusion.  This is especially relevant
    when the maintainer did not heavily participate in the discussion and
    instead left the review to trusted others.

    Do not forget to add trailers such as `Acked-by:`, [...]

And I have only managed to follow that part maybe, probably one single time.

>>     be a big win.
>>
>>     So, I dunno.
>>
>> In conclusion for now: I dunno. :)
>
> I do not know either, but if we agree that everybody should do so
> themselves and I should refrain from applying the ones that lack
> Acks, I can adjust.  There will be lot of unapplied patches left on
> the mailing list initially until the contributors adjust their
> behaviour, but in the long run it may be beneficial?

Okay, if the proposal is to *not* e.g. graduate series to `next` that
haven’t applied the acks etc. then I understand how it will likely lead
to some stalls until people adjust.

To be clear, I imagine this is how it would play out:

• The series in itself is ready for `next` and has no unapplied acks
  etc.: it graduates to `next`
• The series in itself is ready for `next` but has unapplied acks etc.:
  it does not graduate to `next` since the contributor should send a new
  version with the trailer changes

***

There is also the paragraph previous to the trailer one:

    After the list reached a consensus that it is a good idea to apply the
    patch, re-send it with "To:" set to the maintainer{current-maintainer}
    and "cc:" the list{git-ml} for inclusion.  This is especially relevant
    when the maintainer did not heavily participate in the discussion and
    instead left the review to trusted others.

    Do not forget to add trailers such as `Acked-by:`, [...]

And I have only managed to follow that part maybe, probably one single time.

      parent reply	other threads:[~2026-05-20  7:11 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-05 20:45 [PATCH] doc: add caveat about turning off commit-graph kristofferhaugsbakk
2026-05-06 13:59 ` Derrick Stolee
2026-05-07 14:30   ` Kristoffer Haugsbakk
2026-05-07 18:03     ` Derrick Stolee
2026-05-07 18:20 ` [PATCH v2] " kristofferhaugsbakk
2026-05-07 18:59   ` Derrick Stolee
2026-05-07 19:42   ` [PATCH v3] " kristofferhaugsbakk
2026-05-07 19:56     ` Derrick Stolee
2026-05-07 21:14       ` Kristoffer Haugsbakk
2026-05-11  1:16         ` Junio C Hamano
2026-05-11  8:29           ` Oswald Buddenhagen
2026-05-20  7:10           ` Kristoffer Haugsbakk [this message]

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=344cc7d8-4ab3-40f4-9564-80e5888c5bc9@app.fastmail.com \
    --to=kristofferhaugsbakk@fastmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=oswald.buddenhagen@gmx$(echo .)de \
    --cc=stolee@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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