Re: [PATCH v2 01/11] builtin/pack-objects.c: change check_pbase_path() to use ALLOC_GROW()

From: Jeff King <peff@peff.net>
Date: Mar 1, 2014, 07:07 UTC
Message-ID: <20140301070758.GE20397@sigill.intra.peff.net>
In-Reply-To: <xmqqfvn3ukjs.fsf@gitster.dls.corp.google.com>

On Fri, Feb 28, 2014 at 11:03:19AM -0800, Junio C Hamano wrote:

Show 14 quoted lines

> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
> > So my vote is that the patches are OK the way Dmitry wrote them (mind, I
> > have only read through 05/11 so far).
> 
> Seconded ;-)
> 
> By the way, I do not like these long subjects.  "change" is a
> redundant word when one sends a patch---as all patches are about
> changing something.
> 
> 	Subject: builtin/pack-objects.c: use ALLOC_GROW() in check_pbase_path()
> 
> would be a lot more appropriate for "git shortlog" consumption.

I would actually go one step further and drop or shorten the filename in the subject. It is very long, it is already easy to see which file was changed from the diffstat, and it doesn't give any useful context for other parts of the subject.

I really like the "foo:" convention for starting a subject line, because it immediately makes clear what area you are working in without having to waste space on English conjunctions or prepositions. But it does not have to be a filename. It can be a subsystem, a command, a function, an area of the project, or anything that gives context to the rest of the line.

So I would suggest one of:

  Subject: use ALLOC_GROW() in check_pbase_path()

    Talking about the filename is redundant; there's only one
    check_pbase_path.

  Subject: check_pbase_path: use ALLOW_GROW

    Even shorter.

  Subject: builtin/pack-objects.c: use ALLOC_GROW

    This one implies to me that the point of the commit is to convert
    the whole file to use ALLOC_GROW where appropriate, not just that
    function (even if that function may be the only spot changed).

I'd probably not use:

  Subject: pack-objects: use ALLOC_GROW

as the scope is not about the command, but about the C file.

I realize that I just bikeshedded on subject lines for half a page, and part of me wants to go kill myself in shame. But I feel like I see the technique misapplied often enough that maybe some guidance is merited. Feel free to ignore. :)

-Peff