Re: [Qemu-devel] [PATCH] fix BCD mask for date (Solaris 2.5 guest hang fix)

[Andreas Färber]

Am 23.04.2012 20:28, schrieb Artyom Tarasenko:
> On Mon, Apr 23, 2012 at 7:34 PM, Andreas Färber <address@hidden> wrote:
>> Am 23.04.2012 18:48, schrieb Artyom Tarasenko:
>>> On Mon, Apr 23, 2012 at 5:18 PM, Andreas Färber <address@hidden> wrote:
>>>> Ideally the patch would also indicate that it's about "m48t59: ".
>>>
>>> You mean, like "[PATCH,m48t59] fix BCD mask for date (Solaris 2.5
>>> guest hang fix)" ?
>>
>> No, within [] it's not preserved for git. I meant like
>> "[PATCH] m48t59: Fix BCD mask for date" or
>> "[PATCH] hw/m48t59: Fix BCD mask for date" or something like that.
>>
>> Makes it easier to spot patches on the list that are of interest - in
>> this case m48t59 affects ppc/PReP too, not just Solaris.
> 
> Right. But on the other hand, it makes obvious the use case it's
> fixing. If you discover that the patch breaks PReP, and the code needs
> to be changed, you'll immediately know what use case might be affected
> by the change.

My point was more that a) if you indicate what device it affects then we
can ignore it for supported downstream x86_64 KVM configs and b) if I
understand that it affects PReP then as maintainer I might want to
review and test it there as well *before* it gets applied and possibly
breaks something (generally speaking). m48t59 has no official maintainer
though, so any committer including Blue may commit it as long as `make
check` passes, for which we apparently don't have conclusive tests for
neither sparc nor ppc, therefore my request.

I'd personally expect consequences of a fix to go into the commit body
but that may be biased...

> Also, what we currently have:
>  git log --pretty=short hw/m48t59.c
> shows that most commits (including the two previous ones) don't have
> the m48t59 prefix.
> Maybe we need a new coding convention for this?

We have, cf. http://wiki.qemu.org/Contribute/SubmitAPatch:

Write a good commit message. QEMU follows the usual standard for git
commit messages: the first line (which becomes the email subject line)
is "subsystem: single line summary of change". Then there is a blank
line and a more detailed description of the patch, another blank and
your Signed-off-by: line. Don't include comments like "This is a
suggestion for fixing this bug" (they can go below the "---" line in the
email so they don't go into the final commit message).

My interpretation of subsystem would be "m48t59", others prefer the
exact filename "hw/m48t59.c" or something in between, but some form of
subsystem indication is expected when applicable.

If you look at qemu-devel or `git log`, most recent patches and commits
stick to the convention.

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg