Messages in this thread | | | Date | Wed, 15 Apr 2009 22:07:24 +0200 | From | Ingo Molnar <> | Subject | Re: Fix quilt merge error in acpi-cpufreq.c |
| |
* Linus Torvalds <torvalds@linux-foundation.org> 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 <mingo@elte.hu> | 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 instead:
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 glance.
I could have concatenated the first subject line with the impact information:
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.
Ingo
| |