Re: [Qemu-devel] [PATCH 1/4] Clean up wrong usage of FALSE and TRUE in places that use bool from stdbool.h

[Jafar Abdi]

Dear Eric Blake,

Thank you very much for your review I learned a lot from it, I did what you
told me (hopefully I didn't forget something) and send V2 of the patch

Again thank you for your time,

best regards,

Jafar Abdi

On Fri, 22 Mar 2019 at 20:39, Eric Blake <address@hidden> wrote:

> On 3/22/19 7:05 AM, JafarAbdi wrote:
> > Signed-off-by: JafarAbdi <address@hidden>
> > ---
>
> Thank you for your submission - it looks like your first contribution to
> qemu.  However, I have some suggestions that will make it easier to get
> your patches applied:
>
> - Use a cover letter. Since you sent a series, including a 0/4 cover
> letter makes it easier to reply to the series as a whole, and easier for
> our continuous integration tools to run tests on your patches (for a
> single patch, a cover letter is optional).
>
> - Use distinct subject lines. Reusing the same subject line for four
> different patches doesn't help anyone distinguish which patch to
> backport in isolation (if only one of the four needs to be applied to a
> downstream port).
>
> - Use shorter subject lines. Try to stick to around 60 characters, and
> using a 'topic: Description' layout (you can view git log of the file
> you are touching to see topics that have been chosen in the past for
> that file; for example, THIS patch would be better titled
>   authz: fix usage of bool
> then use the body to explain how TRUE/FALSE are for glib's (lame)
> 'gboolean' type only, and everywhere else we prefer <stdbool's> spelling.
>
> - The subject line should say "what" (which you vaguely did, other than
> missing the topic part), but the commit body should say "why" (what
> problems are you fixing, why should a maintainer include your patch,
> etc). You had no commit body other than your Sign-off.
>
> - Your Sign off should be a legal name. I'm not one to argue whether
> 'JafarAbdi' is (one of) your legal name(s), but without a space it looks
> suspicious as if it might be your login name instead, so I'm wondering
> if you need to tweak your git settings so that you show up as 'Jafar
> Abdi' or whatever other spelling you prefer to go by on legal documents.
>
> >  authz/listfile.c |     2 +-
> >  qemu.config      |     2 +
> >  qemu.creator     |     1 +
> >  qemu.files       | 28238
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  qemu.includes    |  2568 +++++
> >  5 files changed, 30810 insertions(+), 1 deletion(-)
> >  create mode 100644 qemu.config
> >  create mode 100644 qemu.creator
> >  create mode 100644 qemu.files
> >  create mode 100644 qemu.includes
>
> - Your diffstat is highly unusual. (Thankfully, your email did not
> actually include 28238 bogus lines of changes to qemu.files, or it would
> have been a lot larger than 6k). Use 'git rebase -i' to polish your
> patches before sending, so that you don't have to touch up your emails
> to drop cruft that shouldn't be in the commit after all.
>
> - You failed to cc: the maintainer for the file that your patch touches
> (for authz, scripts/get_maintainer.pl reports that Dan Berrange should
> be listed in cc; which I've done). Git can be configured to auto-cc
> maintainers without you having to think about it.
>
> - More patch submission tips at:
> https://wiki.qemu.org/Contribute/SubmitAPatch
>
> Good luck!
>
> >
> > diff --git a/authz/listfile.c b/authz/listfile.c
> > index d457976..03eaf46 100644
> > --- a/authz/listfile.c
> > +++ b/authz/listfile.c
> > @@ -238,7 +238,7 @@ qauthz_list_file_init(Object *obj)
> >
> >      authz->file_watch = -1;
> >  #ifdef CONFIG_INOTIFY1
> > -    authz->refresh = TRUE;
> > +    authz->refresh = true;
> >  #endif
> >  }
> >
> >
>
> At any rate, this code change is correct, so if you submit a v2 with an
> improved commit message, you can add:
>
> Reviewed-by: Eric Blake <address@hidden>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>
>