Re: [PATCH v3 0/3] -Wunreachable-code

From: Jeff King <peff@peff.net>
Date: Mar 18, 2025, 00:18 UTC
Message-ID: <20250318001835.GA1470172@coredump.intra.peff.net>
In-Reply-To: <20250317235329.809302-1-gitster@pobox.com>

On Mon, Mar 17, 2025 at 04:53:26PM -0700, Junio C Hamano wrote:

> As Taylor noticed, we can still help macOS users by first dealing
> with the false positive in the code, and then flip the warning
> option for developers on.

Yeah, this is worth doing.

Show 14 quoted lines

> This is totally offtopic, but I often find the short-log (list of
> commits, grouped by author) in the cover letter very awkward to work
> with.  Between v2 and v3, aside from the NOT_CONSTANT() improvements
> in the patch [2/3] that used to be [3/3], one large change is the
> reordering of the patches but that is not seen in the shortlog (I
> ran "git log --oneline -reverse" to prepare the list of commits in
> the order they are applied to describe them in the above list).
> 
> Jeff King (2):
>   run-command: use errno to check for sigfillset() error
>   config.mak.dev: enable -Wunreachable-code
> 
> Junio C Hamano (1):
>   git-compat-util: add NOT_CONSTANT macro and use it in atfork_prepare()

The re-ordering does appear in the range-diff, if you provide one. But I agree that the organized-by-name shortlog does not make much sense for most series. As a reviewer, I care most about the patches, not the authors.

I make my cover letters with something like this (part of a larger script):

    git format-patch --stdout origin..$topic |
    perl -lne '
      if (/^Subject: (.*)/) {
        $subject = $1;
      }
      elsif ($subject && /^\s+(.*)/) {
        $subject .= " $1";
      }
      elsif ($subject) {
        print $subject;
        $subject = undef;
      }
    ' |
    sed -e 's/\[PATCH /[/' \
        -e 's/]/]:/' \
        -e 's/^/  /'

which yields something like (for the older version of this series):

  [1/3]: config.mak.dev: enable -Wunreachable-code
  [2/3]: run-command: use errno to check for sigfillset() error
  [3/3]: git-compat-util: add NOT_A_CONST macro and use it in atfork_prepare()

Having the correct order and the matching numbering next to each one makes it much easier if you're going to comment on them inline.

The perl in the script above is required to handle rfc822 wrapping / line continuation. I never bothered to implement rfc2047 unquoting. I don't tend to use non-ascii chars in my subject lines. ;)

It would be nice if we had a format-patch option to avoid quoting and wrapping in order to make text processing like this easier.

-Peff