All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Cédric Le Goater" <clg@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	qemu-devel@nongnu.org
Subject: Re: [PATCH 1/2] hw/vfio/migration: Remove unused 'exec/ram_addr.h' header
Date: Wed, 01 Mar 2023 08:47:37 +0100	[thread overview]
Message-ID: <87h6v452w6.fsf@pond.sub.org> (raw)
In-Reply-To: <20230227101325.1ba4f5c4.alex.williamson@redhat.com> (Alex Williamson's message of "Mon, 27 Feb 2023 10:13:25 -0700")

Alex Williamson <alex.williamson@redhat.com> writes:

> On Mon, 27 Feb 2023 16:24:16 +0000
> Alex Bennée <alex.bennee@linaro.org> wrote:
>
>> Alex Williamson <alex.williamson@redhat.com> writes:
>> 
>> > On Mon, 27 Feb 2023 11:32:57 +0100
>> > Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>> >  
>> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>  
>> >
>> > Empty commit logs are a pet peeve of mine, there must be some sort of
>> > motivation for the change, something that changed to make this
>> > possible, or perhaps why this was never necessary.  Thanks,  
>> 
>> I think that is a little unfair for this case as the motivation and
>> action are all covered by the summary line. Granted the overall goal is
>> covered by the cover letter and the following patch however for this
>> patch I would hope its self explanatory.
>
> The commit title only conveys that the include is unused and the action
> to therefore remove it.  The motivation is actually to create target
> independent objects and the specific reasoning here is that not only is
> this include file unnecessary, but forces per target builds.

I've been guilty of submitting patches with just a subject line, too.
E.g. commit 6f1e91f716a (error: Drop superfluous #include
"qapi/qmp/qerror.h"): I could have written that the header's contents
should go away, and the less we include it, the better.  I should have.

> This is exactly the "barrier to entry" I mention to Peter, it's obvious
> to the experts in the crowd, but it provides no underlying direction
> for those who might be more novice.  There is always some reasoning
> that can be included in a commit log beyond the simple "a therefore b"
> of the commit title.  Thanks,

Sometimes, the motivation is so obvious that omitting it for brevity's
sake feels harmless enough.  E.g. for commit 15b7646c7d0 (monitor: Drop
unnecessary includes) it's really just "include less to remake less".

Of course, when you explain motivation in the cover letter, it's almost
certainly best to explain it again in the commit messages.



  reply	other threads:[~2023-03-01  7:48 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-27 10:32 [PATCH 0/2] hw/vfio: Build various target-independent objects once Philippe Mathieu-Daudé
2023-02-27 10:32 ` [PATCH 1/2] hw/vfio/migration: Remove unused 'exec/ram_addr.h' header Philippe Mathieu-Daudé
2023-02-27 13:40   ` Cédric Le Goater
2023-02-27 15:46   ` Alex Williamson
2023-02-27 16:04     ` Peter Maydell
2023-02-27 16:34       ` Alex Williamson
2023-02-27 21:58         ` Philippe Mathieu-Daudé
2023-03-01 19:32       ` Thomas Huth
2023-02-27 16:24     ` Alex Bennée
2023-02-27 17:13       ` Alex Williamson
2023-03-01  7:47         ` Markus Armbruster [this message]
2023-02-27 10:32 ` [PATCH 2/2] hw/vfio: Build various target-independent objects once Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h6v452w6.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=alex.williamson@redhat.com \
    --cc=clg@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.