[lkml]   [2009]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: Fix quilt merge error in acpi-cpufreq.c

* Linus Torvalds <> wrote:

> On Wed, 15 Apr 2009, H. Peter Anvin wrote:
> >
> > "cleanup" is indeed the most common, as it is intended to signify a
> > trivial but nonzero code change. Whether or not it's *correct* is
> > another matter. "build fix" is valid and proper use: it tells that it
> > fixes a compilation error, which succinctly communicates both the
> > priority of the fix and how it needs to be validated.
> Why would that be "proper use"?
> Dammit, if the "build fix" is not obvious from the rest of the
> commit message, there's something wrong.
> And if it _is_ obvious, then the mechanical "Impact:" thing is
> pointless.
> In other words - in neither case does it actually help anything at
> all. It's only distracting noise.

I often skip "Impact: build fix" - when it's obvious from the
subject line or the first sentence of the commit - or if it can be
made obvious by changing the subject line or by changing the first
sentence of the commit.

I add it occasionally, when some other, higher priority principle
makes the changing of the subject line undesired.

For example, yesterday i did this commit:

| commit 27b19565fe4ca5b0e9d2ae98ce4b81ca728bf445
| Author: Ingo Molnar <>
| Date: Tue Apr 14 11:03:12 2009 +0200
| lockdep: warn about lockdep disabling after kernel taint, fix
| Impact: build fix for Sparc and s390
| Stephen Rothwell reported that the Sparc build broke:

I added that 'build fix' impact line for two reasons:

Firstly, because the subject line was inherited from the buggy
commit and the new subject line got a ", fix" postfix. (This
convention seems rather useful at times in shortlogs, see below.)

Secondly, i also added the impact line because i wanted to specify
the architectures affected: Sparc and s390 - this fact was not
obvious from the bug report context which i wanted to preserve to
credit the bug reporter prominently (Stephen found the build error
on Sparc only).

Another option would have been to use this primary subject line

fix build error on Sparc and s390

But IMHO that's a worse subject line. It's more important to keep
the flow of the original change intact. The subject lines cluster up
better in shortlogs or in git logs:

$ gll include/linux/debug_locks.h
27b1956: lockdep: warn about lockdep disabling after kernel taint, fix
9eeba61: lockdep: warn about lockdep disabling after kernel taint

The connection between the two commits is plain obvious, at a

I could have concatenated the first subject line with the impact

27b1956: lockdep: warn about lockdep disabling after kernel taint, fix build error on Sparc and s390

... but this is clearly over-long and dillutes the subject line with
'effect' information.


 \ /
  Last update: 2009-04-15 22:13    [from the cache]
©2003-2014 Jasper Spaans. hosted at Digital Ocean