Re: Fix quilt merge error in acpi-cpufreq.c

[H. Peter Anvin]

Ingo Molnar wrote:
> 
> I got Rusty to use it so i'm to blame for this one. A number of 
> developers/maintainers use it now and they find it useful in a 
> number of circumstances, when used judiciously.
> 
> We are using impact lines to judge "practical impact of a commit". 
> The shorter (while still correct and expressive), the better. We are 
> trying to use it in well-defined cases - but not always.
> 
> Here it helped expose the bogosity of a patch more clearly: the 
> intended impact of "clarifying a confusing API" was not met, and the 
> commit became easier to flame.
> 

It's more than that.

Impact: clarify and extend confusing API

... isn't really an impact as much of an action.  This is part of why
it's kind of bogus in this case.  I'm not particularly blaming Rusty on
that, I personally find it's actually really hard to write Impact:
lines, exactly because it is hard to think about the consequences of a
patch -- and it's even harder when the patch is someone else's.

I generally find I spend about three times longer reviewing the same
amount of code from someone who has written Impact: lines on their
patches, mostly because it means they have thought about it.  It does
get reflected in the rest of the patch.  Also, if they aren't there, I
do end up writing them, and quite often find issues in the process.

Of course, in isolation -- and when wrong -- the Impact: lines look
rather stupid.  However, as part of a bigger workflow we've found that
they actually work, mostly because they do work as a forcing function on
both the maintainer and the submitter.

Nothing is perfect, sadly.  It just does seem to be a net win.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/