| Message ID | 1553256353-29753-1-git-send-email-cafer.abdi@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] Clean up wrong usage of FALSE and TRUE in places that use bool from stdbool.h | expand |
On 3/22/19 7:05 AM, JafarAbdi wrote: > Signed-off-by: JafarAbdi <cafer.abdi@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 <eblake@redhat.com>
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 <eblake@redhat.com> wrote: > On 3/22/19 7:05 AM, JafarAbdi wrote: > > Signed-off-by: JafarAbdi <cafer.abdi@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 <eblake@redhat.com> > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3226 > Virtualization: qemu.org | libvirt.org > >
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 }
Signed-off-by: JafarAbdi <cafer.abdi@gmail.com> --- 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