git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Comment trailers vs. bracketed lines

From
Junio C Hamano <gitster@pobox.com>
Date
May 7, 2025, 19:20 UTC
Message-ID
<xmqqv7qcb52r.fsf@gitster.g>
In-Reply-To
<ae56dc14-bbae-4d5e-a890-20735131d484@app.fastmail.com>
"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
Show 27 quoted lines
>> I think in the usual trailer order, it would be:
>>
>>   Signed-off-by: Ramsay
>>   [jk: add commit message]
>>   Signed-off-by: me
>>
>> but I didn't want to forge his S-o-b without asking first.
>
> I’ve seen those brackets in the log.  They used to happen with some
> regularity.  At first it made sense since you need a free-form area to
> both comment and tell everyone that you left the comment.  And a trailer
> doesn’t make sense for that, I thought.[1]
>
> But thinking about the signoff requirement: you already have all the
> information you need from the next trailer, namely the signoff.  In
> other words this:
>
>     [kh: Added tests]
>     Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> Has the same information as this:
>
>     Comment: Added tests
>     Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> Because the signoff order tells you who left the comment.  So I was
> wondering to myself why this uniform approach wasn’t used.

Simply because we already saw another project like the kernel use the [initial: comment] convention, I think. The "25% rule" was originally added specifically to accomdate this kind of comments frequently used in the kernel project, if I am not mistaken.

We see too many 'jk' and 'js' in the project so the initial may not be all that meaningful if added and it usually is obvious who did what without, but even with three letter initial,

    [khh: Added tests]
    Comment: Added tests

the existing convention is still shorter than with your "Comment: " prefix ;-)

Previous: Jeff King
Message 22 of 22 in “builtin/credential-cache--daemon: fix error when "exit"ing on Cygwin”
  1. builtin/credential-cache--daemon: fix error when "exit"ing on CygwinPatrick Steinhardt, Oct 16, 2024
  2. Jeff KingOct 16, 2024
  3. Jeff KingOct 16, 2024
  4. Ramsay JonesOct 16, 2024
  5. Taylor BlauOct 16, 2024
  6. Jeff KingOct 17, 2024
  7. Patrick SteinhardtOct 17, 2024
  8. Ramsay JonesOct 17, 2024
  9. Patrick SteinhardtOct 18, 2024
  10. Taylor BlauOct 17, 2024
  11. Ramsay JonesOct 16, 2024
  12. builtin/credential-cache--daemon: fix error when "exit"ing on CygwinPatrick Steinhardt, Oct 18, 2024
  13. credential-cache: treat ECONNABORTED like ECONNRESETJeff King, Oct 18, 2024
  14. Patrick SteinhardtOct 18, 2024
  15. Ramsay JonesOct 18, 2024
  16. Taylor BlauOct 18, 2024
  17. Kristoffer HaugsbakkOct 18, 2024
  18. Jeff KingOct 19, 2024
  19. Comment trailers vs. bracketed linesKristoffer Haugsbakk, Oct 20, 2024
  20. Kristoffer HaugsbakkMay 7, 2025
  21. Jeff KingMay 8, 2025
  22. Junio C HamanoMay 7, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.