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

Linus Torvalds
2009-04-16T02:39:38Z

On Thu, 16 Apr 2009, Rusty Russell wrote:
>
> Side note: I really prefer to see the compile error output in this case: great
> for googling. It annoys me when people skip this.

I do agree that that's generally a good idea, although I will tend to edit
it down when people send in 20 lines of compile errors. Nobody cares at
that point, and it doesn't add value. Give the first few errors, not a
whole log-ful of them.

I also ask that people edit away the irrelevant site-specific parts. I do
that editing myself when I notice, but in case it goes through somebody
who doesn't, or in case I don't notice, look which one is more readable
and informative:

Fix the following warning:

fs/fuse/file.c: In function 'fuse_direct_io':
fs/fuse/file.c:1002: warning: passing argument 3 of 'fuse_get_user_pages' from incompatible pointer type

or

Fix staging/rt28x0 printk format warnings:

linux-next-20090209/drivers/staging/rt2860/common/spectrum.c:1599: warning: format '%d' expects type 'int', but argument 3 has type
linux-next-20090209/drivers/staging/rt2860/rt_linux.c:857: warning: format '%d' expects type 'int', but argument 3 has type 'long u

and realize that the "linux-next-20090209/" part doesn't help (and that's
a pretty _mild_ example of this particular issue, we have tons of examples
of absolute pathname prefixes like "/home/jeremy/hg/xen/paravirt/linux/",
so if you look at the log in even a 100-character wide window, you hardly
see any of the actual _warning_ at all, it's mostly all just pathnames ;)

> Let's get concrete. Here's the top 3 non-merge commits in gitk:
>
> ALSA: hda - Fix the cmd cache keys for amp verbs
>
> Fix the key value generation for get/set amp verbs. The upper bits of
> the parameter have to be combined with the verb value to be unique for
> each direction/index of amp access.
>
> This fixes the resume problem on some hardwares like Macbook after
> the channel mode is changed.
>
> I have no idea what this patch does. It seems to be a fix; what are the
> symptoms of the problem, and how long has it been there?

Oh, I'm absolutely not going to claim that we should not improve on
changelogs in general. But if you actually look at that commit, I would
argue that the commit message together with the patch is likely not that
bad to understand for somebody who understands the hardware well enough
for the code to make sense in the first place!

So could it be improved? I suspect _every_ commit message can be improved.
But do you really think that an "Impact" statment would be the big deal?
Or, in fact, any kind of "fixed message format".

> ALSA: add missing definitions(letters) to HD-Audio.txt
>
> impact: Add missing definitions(letters).

And here, the "impact:" part certainly in no way improves anything.

> ALSA: sound/pci: use memdup_user()
>
> Remove open-coded memdup_user().
>
> Again, the body seems gratuitous.

And yes, I agree that in many cases you don't need a body at all. If the
patch is trivial, and the subject tells it all, then why bother with a
body? We've got a fair number of those.

That said, I'd rather have a few redundant (but still readable) bodies.
And as mentioned, I don't think a "perfect" changelog exists. Some people
will always want more, others will think it's unnecessary. And even if a
perfect changelog existed, we'll never have the perfect people who write
them.

But machine-readability should be the _last_ thing we look for.

Linus