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 <ebl...@redhat.com> wrote: > On 3/22/19 7:05 AM, JafarAbdi wrote: > > Signed-off-by: JafarAbdi <cafer.a...@gmail.com> > > --- > > 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 <ebl...@redhat.com> > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3226 > Virtualization: qemu.org | libvirt.org > >
