On Fri, Feb 28, 2014 at 11:03:19AM -0800, Junio C Hamano wrote:
> Michael Haggerty <mhag...@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
So I would suggest one of:
Subject: use ALLOC_GROW() in check_pbase_path()
Talking about the filename is redundant; there's only one
Subject: check_pbase_path: use ALLOW_GROW
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. :)
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html