qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Juan Quintela <quintela@redhat.com>
Cc: qemu-devel@nongnu.org,  richard.henderson@linaro.org,
	pbonzini@redhat.com,  kwolf@redhat.com,  hreitz@redhat.com,
	imp@bsdimp.com,  kevans@freebsd.org,  berrange@redhat.com,
	groug@kaod.org,  qemu_oss@crudebyte.com,  mst@redhat.com,
	philmd@linaro.org,  peter.maydell@linaro.org,
	 alistair@alistair23.me, jasowang@redhat.com,
	 jonathan.cameron@huawei.com, kbastian@mail.uni-paderborn.de,
	 dgilbert@redhat.com, michael.roth@amd.com,  kkostiuk@redhat.com,
	 tsimpson@quicinc.com, palmer@dabbelt.com,
	 bin.meng@windriver.com,  qemu-block@nongnu.org,
	qemu-arm@nongnu.org,  qemu-riscv@nongnu.org
Subject: Re: [PATCH v5 04/20] scripts/clean-includes: Improve --git commit message
Date: Thu, 02 Feb 2023 10:00:49 +0100	[thread overview]
Message-ID: <87k010phku.fsf@pond.sub.org> (raw)
In-Reply-To: <877cx11lny.fsf@secure.mitica> (Juan Quintela's message of "Wed,  01 Feb 2023 15:54:09 +0100")

Juan Quintela <quintela@redhat.com> writes:

> Markus Armbruster <armbru@redhat.com> wrote:
>> Juan Quintela <quintela@redhat.com> writes:
>>
>>> Markus Armbruster <armbru@redhat.com> wrote:
>>>> Juan Quintela <quintela@redhat.com> writes:
>>>>
>>>>> Markus Armbruster <armbru@redhat.com> wrote:
>>>>>> The script drops #include "qemu/osdep.h" from headers.  Mention it in
>>>>>> the commit message it uses for --git.
>>>>>>
>>>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>>>> ---
>>>>>>  scripts/clean-includes | 4 ++--
>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/scripts/clean-includes b/scripts/clean-includes
>>>>>> index f0466a6262..f9722c3aec 100755
>>>>>> --- a/scripts/clean-includes
>>>>>> +++ b/scripts/clean-includes
>>>>>> @@ -193,8 +193,8 @@ if [ "$GIT" = "yes" ]; then
>>>>>>      git commit --signoff -F - <<EOF
>>>>>>  $GITSUBJ: Clean up includes
>>>>>>  
>>>>>> -Clean up includes so that osdep.h is included first and headers
>>>>>> -which it implies are not included manually.
>>>>>> +Clean up includes so that qemu/osdep.h is included first in .c, and
>>>>>> +not in .h, and headers which it implies are not included manually.
>>>>>
>>>>> I give a tree.
>>>>>
>>>>> Clean up includes so qemu/osdep.h is never used in .h files.  It makes
>>>>> sure that qemu/osdep.h is only used in .c files.  Once there, it assures
>>>>> that .h files already included in qemu/osdep.h are not included a second
>>>>> time on the .c file.
>>>>>
>>>>> What do you think?
>>>>
>>>> Neglects to mention qemu/osdep.h goes first in .c.
>>>
>>> /me tries again
>>>
>>> What about:
>>>
>>> The file qemu/osdep.h should only be included in .c files.  And it has
>>> to be the first included file.
>>
>> Suggest "has to be included first."
>
> Ok to this change.
>
>>
>>> This script does several things:
>>> - Remove qemu/osdep.h from .h files.
>>
>> Correct.  Could say "inclusion of qemu/osdep.h"
>
> I try to minimize whatever word that "includes" "includ*" (pun intended).
>
>>> - If qemu/osdep.h is included in a .c file it is moved to the first
>>>   included position if it is anywhere else.
>>
>> Not quite.  The script ensures all the .c include it, and include it
>> first.
>
> Oh, then it is easier.
>
> - It ensures that qemu/osdep.h is the first included file in all .c files.
>
>>> - Remove from .h files all include files that are already present in
>>>   qemu/osdep.h.
>>
>> They're removed from .h, too.
>
> Ah, didn't know this bit.
>
>> Sure you want to continue wordsmithing?  ;)
>
> Yeap, I *hate* error messages that I can't parse (or have to read it ten
> times before I understand them).
>
> So, we end with:
>
> The file qemu/osdep.h should only be included in .c files.  And it has
> to be included first.
>
> This script does three things:
> - Remove qemu/osdep.h from .h files.
> - It ensures that qemu/osdep.h is the first included file in all .c files.
> - Include files contained in qemu/osdep.h are removed form all .c and .h
>   files.
>
> Is this better?

It's less terse.  Fine with me.  The mix of passive and active voice
feels a bit awkward, though.  Another try:

  All .c should include qemu/osdep.h first.  This script performs three
  related cleanups:

  * Ensure .c files include qemu/osdep.h first.
  * Including it in a .h is redundant, since the .c  already includes
    it.  Drop such inclusions.
  * Likewise, including headers qemu/osdep.h includes is redundant.
    Drop these, too.



  reply	other threads:[~2023-02-02  9:02 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-30 13:21 [PATCH v5 00/20] Clean up includes Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 01/20] scripts/clean-includes: Fully skip / ignore files Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 02/20] scripts/clean-includes: Don't claim duplicate headers found when not Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 03/20] scripts/clean-includes: Skip symbolic links Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 04/20] scripts/clean-includes: Improve --git commit message Markus Armbruster
2023-01-30 14:12   ` Juan Quintela
2023-02-01 12:49     ` Markus Armbruster
2023-02-01 13:14       ` Juan Quintela
2023-02-01 13:44         ` Markus Armbruster
2023-02-01 14:54           ` Juan Quintela
2023-02-02  9:00             ` Markus Armbruster [this message]
2023-02-02 10:54               ` Juan Quintela
2023-01-30 13:21 ` [PATCH v5 05/20] bsd-user: Clean up includes Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 06/20] crypto: " Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 07/20] hw/cxl: " Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 08/20] hw/input: " Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 09/20] hw/tricore: " Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 10/20] qga: " Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 11/20] migration: " Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 12/20] net: " Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 13/20] target/hexagon: " Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 14/20] riscv: " Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 15/20] block: " Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 16/20] accel: " Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 17/20] Fix non-first inclusions of qemu/osdep.h Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 18/20] Don't include headers already included by qemu/osdep.h Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 19/20] 9p: Drop superfluous include of linux/limits.h Markus Armbruster
2023-01-30 13:21 ` [PATCH v5 20/20] Drop duplicate #include Markus Armbruster

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=87k010phku.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=alistair@alistair23.me \
    --cc=berrange@redhat.com \
    --cc=bin.meng@windriver.com \
    --cc=dgilbert@redhat.com \
    --cc=groug@kaod.org \
    --cc=hreitz@redhat.com \
    --cc=imp@bsdimp.com \
    --cc=jasowang@redhat.com \
    --cc=jonathan.cameron@huawei.com \
    --cc=kbastian@mail.uni-paderborn.de \
    --cc=kevans@freebsd.org \
    --cc=kkostiuk@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=mst@redhat.com \
    --cc=palmer@dabbelt.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=qemu_oss@crudebyte.com \
    --cc=quintela@redhat.com \
    --cc=richard.henderson@linaro.org \
    --cc=tsimpson@quicinc.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).