[Qemu-devel] Question: Perfecting checkpatch.pl

[Qemu-devel] Question: Perfecting checkpatch.pl

[Aleksandar Markovic]

Perhaps checkpatch.pl should make sure the commit messages do not contain "address@hidden" and "Reviewed-off-by"?

$ git log | grep address@hidden
    Signed-off-by: Michael S. Tsirkin <address@hidden>
    Signed-off-by: Ronald Hecht <address@hidden>
    Signed-off-by: Justin Chevrier <address@hidden>
    Signed-off-by: Justin Chevrier <address@hidden>
    Signed-off-by: Justin Chevrier <address@hidden>

$ git log | grep Reviewed-off-by
    Reviewed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
    Reviewed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Aleksandar

Re: [Qemu-devel] Question: Perfecting checkpatch.pl

[Paolo Bonzini]

On 30/10/2018 13:35, Aleksandar Markovic wrote:
> Perhaps checkpatch.pl should make sure the commit messages do not contain "address@hidden" and "Reviewed-off-by"?
> 
> $ git log | grep address@hidden
>     Signed-off-by: Michael S. Tsirkin <address@hidden>
>     Signed-off-by: Ronald Hecht <address@hidden>
>     Signed-off-by: Justin Chevrier <address@hidden>
>     Signed-off-by: Justin Chevrier <address@hidden>
>     Signed-off-by: Justin Chevrier <address@hidden>
> 
> $ git log | grep Reviewed-off-by
>     Reviewed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
>     Reviewed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Sure, both are good ideas.  Can you write a patch for it?

Paolo

Re: [Qemu-devel] Question: Perfecting checkpatch.pl

[Philippe Mathieu-Daudé]

On 30/10/18 13:35, Aleksandar Markovic wrote:
> Perhaps checkpatch.pl should make sure the commit messages do not contain "address@hidden" and "Reviewed-off-by"?
> 
> $ git log | grep address@hidden
>      Signed-off-by: Michael S. Tsirkin <address@hidden>
>      Signed-off-by: Ronald Hecht <address@hidden>
>      Signed-off-by: Justin Chevrier <address@hidden>
>      Signed-off-by: Justin Chevrier <address@hidden>
>      Signed-off-by: Justin Chevrier <address@hidden>

Having a check for that is a good idea, but I don't think checkpatch 
would help, these incorrect emails only appear once maintainer applied 
patches, maybe the good place to check would be Peter's magic scripts, see:
https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/apply-pullreq?id=eeb001fcc1aabd8f077cd2846724120a3aa8f962#n118

Those emails also entered the tree:

hw/usb/hcd-ehci.c:4: * Copyright(c) 2008  Emutex Ltd. (address@hidden)
include/qemu/pmem.h:6: * Author: Haozhong Zhang <address@hidden>

> 
> $ git log | grep Reviewed-off-by
>      Reviewed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
>      Reviewed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> Aleksandar
> 

Re: [Qemu-devel] Question: Perfecting checkpatch.pl

[Aleksandar Markovic]

> From: Paolo Bonzini <pbonzini@redhat.com>
>
> Sure, both are good ideas.  Can you write a patch for it?

I would rather not, I prefer to see it as a black box.

Aleksandar

Re: [Qemu-devel] Question: Perfecting checkpatch.pl

[Peter Maydell]

On 30 October 2018 at 14:48, Aleksandar Markovic <amarkovic@wavecomp.com> wrote:
>
>> From: Paolo Bonzini <pbonzini@redhat.com>
>>
>> Sure, both are good ideas.  Can you write a patch for it?
>
> I would rather not, I prefer to see it as a black box.

So do most of the rest of us :-)  This is why sometimes
it does not have all the features that might seem nice for
it to have...

thanks
-- PMM

Re: [Qemu-devel] Question: Perfecting checkpatch.pl

[Daniel P. Berrangé]

On Tue, Oct 30, 2018 at 12:35:15PM +0000, Aleksandar Markovic wrote:
> Perhaps checkpatch.pl should make sure the commit messages do not contain "address@hidden" and "Reviewed-off-by"?
> 
> $ git log | grep address@hidden
>     Signed-off-by: Michael S. Tsirkin <address@hidden>
>     Signed-off-by: Ronald Hecht <address@hidden>
>     Signed-off-by: Justin Chevrier <address@hidden>
>     Signed-off-by: Justin Chevrier <address@hidden>
>     Signed-off-by: Justin Chevrier <address@hidden>

This makes sense...

> 
> $ git log | grep Reviewed-off-by
>     Reviewed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
>     Reviewed-off-by: Anthony Liguori <aliguori@us.ibm.com>

but I'm not convinced it is worth doing this, because the set of
mistakes people have made is enourmous, so it turns into a game
of whack-a-mole.

 git log | grep -- '-by:' | sed -e 's/-by:.*/-by/' | sed -e 's/ //g' | sort | uniq -c
   3127 Acked-by
      1 ACKed-by
      1 Acked-off-by
      5 Analyzed-by
      3 Approximately-suggested-by
      3 Co-authored-by
      1 Edited-by
      2 eviewed-by
      1 Fine-with-me'd-by
      1 Fix-suggested-by
      3 Found-by
      1 Inspired-by
      1 Introduced-by
      1 Original-patch-by
      1 Problem-spotted-by
      2 Proposed-by
      2 Rebased-by
      1 Release-acked-by
      1 Reported-and-analyzed-by
      2 Reported-and-tested-by
      1 Reported-and-Tested-by
   1834 Reported-by
      1 Reportyed-by
      7 Requested-by
      2 Reveiwed-by
      1 Revieed-by
      2 Revieved-by
      4 Reviewd-by
  30991 Reviewed-by
      2 Reviewed-off-by
      3 Reviwed-by
      1 Rported-by
      1 Sgined-off-by
      1 Sigend-off-by
      1 Signed-by
      1 Signed-of-by
  97837 Signed-off-by
      1 Signer-off-by
      4 Singed-off-by
      1 Spotted-by
    660 Suggested-by
   1951 Tested-by
      1 Tested-off-by
      1 xSigned-off-by


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] Question: Perfecting checkpatch.pl

[Peter Maydell]

On 31 October 2018 at 12:00, Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Tue, Oct 30, 2018 at 12:35:15PM +0000, Aleksandar Markovic wrote:
>> Perhaps checkpatch.pl should make sure the commit messages do not contain "address@hidden" and "Reviewed-off-by"?
>>
>> $ git log | grep address@hidden
>>     Signed-off-by: Michael S. Tsirkin <address@hidden>
>>     Signed-off-by: Ronald Hecht <address@hidden>
>>     Signed-off-by: Justin Chevrier <address@hidden>
>>     Signed-off-by: Justin Chevrier <address@hidden>
>>     Signed-off-by: Justin Chevrier <address@hidden>
>
> This makes sense...
>
>>
>> $ git log | grep Reviewed-off-by
>>     Reviewed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
>>     Reviewed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>
> but I'm not convinced it is worth doing this, because the set of
> mistakes people have made is enourmous, so it turns into a game
> of whack-a-mole.

You could have checkpatch whitelist the sensible ones:

>    3127 Acked-by
>    1834 Reported-by
>   30991 Reviewed-by
>   97837 Signed-off-by
>     660 Suggested-by
>    1951 Tested-by

and warn about the rest...

thanks
-- PMM

Re: [Qemu-devel] Question: Perfecting checkpatch.pl

[Daniel P. Berrangé]

On Wed, Oct 31, 2018 at 01:27:15PM +0000, Peter Maydell wrote:
> On 31 October 2018 at 12:00, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > On Tue, Oct 30, 2018 at 12:35:15PM +0000, Aleksandar Markovic wrote:
> >> Perhaps checkpatch.pl should make sure the commit messages do not contain "address@hidden" and "Reviewed-off-by"?
> >>
> >> $ git log | grep address@hidden
> >>     Signed-off-by: Michael S. Tsirkin <address@hidden>
> >>     Signed-off-by: Ronald Hecht <address@hidden>
> >>     Signed-off-by: Justin Chevrier <address@hidden>
> >>     Signed-off-by: Justin Chevrier <address@hidden>
> >>     Signed-off-by: Justin Chevrier <address@hidden>
> >
> > This makes sense...
> >
> >>
> >> $ git log | grep Reviewed-off-by
> >>     Reviewed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> >>     Reviewed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> >
> > but I'm not convinced it is worth doing this, because the set of
> > mistakes people have made is enourmous, so it turns into a game
> > of whack-a-mole.
> 
> You could have checkpatch whitelist the sensible ones:
> 
> >    3127 Acked-by
> >    1834 Reported-by
> >   30991 Reviewed-by
> >   97837 Signed-off-by
> >     660 Suggested-by
> >    1951 Tested-by
> 
> and warn about the rest...

I guess warning would be ok - I don't want to discourage people from
being creative and "singing off" their patches :-)

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|