lkml.org 
[lkml]   [2009]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: 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


    \
     
     \ /
      Last update: 2009-04-15 22:13    [W:0.038 / U:29.416 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site