qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] scripts: let checkpatch.pl process an entire GI


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH] scripts: let checkpatch.pl process an entire GIT branch
Date: Tue, 12 Sep 2017 18:14:57 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1

On 12/09/2017 18:12, Daniel P. Berrange wrote:
> On Tue, Sep 12, 2017 at 05:52:18PM +0200, Paolo Bonzini wrote:
>> On 12/09/2017 12:46, Daniel P. Berrange wrote:
>>> Currently before submitting a series, devs should run checkpatch.pl
>>> across each patch to be submitted. This can be automated using a
>>> command such as:
>>>
>>>   git rebase -i master -x 'git show | ./scripts/checkpatch.pl -'
>>>
>>> This is rather long winded to type, so this patch introduces a new
>>> flag '--branch' to checkpatch.pl which instructs it to check every
>>> patch on the current GIT branch.
>>
>> Great idea, though I'm not sure about having a default.  And to keep it
>> easy to invoke, having a sole argument that ends with ".." might DWIM
>> and enable --branch too...
> 
> I think it is beneficial to have a default, as I figure the majority
> of contributors are working on a branch that's rebased against master..
> Half as many characters to type in the common case :-)

With the DWIM option "--branch" and "master.." are exactly the same
length. :)

> Sometimes people might write patches against a particular subsystem
> staging branch (eg kevin/block), but I don't think there's downside
> in assuming 'master..' by default.

What about "origin/master.." instead?

Paolo

>>
>> Paolo
>>
>>> For example:
>>>
>>>     $ ./scripts/checkpatch.pl --branch
>>>     total: 0 errors, 0 warnings, 297 lines checked
>>>
>>>     b886d352a2bf58f0996471fb3991a138373a2957 has no obvious style problems 
>>> and is ready for submission.
>>>     total: 0 errors, 0 warnings, 182 lines checked
>>>
>>>     2a731f9a9ce145e0e0df6d42dd2a3ce4dfc543fa has no obvious style problems 
>>> and is ready for submission.
>>>     total: 0 errors, 0 warnings, 102 lines checked
>>>
>>>     11844169bcc0c8ed4449eb3744a69877ed329dd7 has no obvious style problems 
>>> and is ready for submission.
>>>
>>> By default it checks every patch identified by 'master..', however,
>>> an alternative origin can be given if desired, if the current branch
>>> is rebased to another non-master branch:
>>>
>>>     $ ./scripts/checkpatch.pl --branch somebranch..
>>>
>>> Signed-off-by: Daniel P. Berrange <address@hidden>
>>> ---
>>>  scripts/checkpatch.pl | 97 
>>> +++++++++++++++++++++++++++++++++++++--------------
>>>  1 file changed, 71 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index fa478074b8..f8d080441f 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -19,6 +19,8 @@ my $quiet = 0;
>>>  my $tree = 1;
>>>  my $chk_signoff = 1;
>>>  my $chk_patch = 1;
>>> +my $chk_branch = 0;
>>> +my $revlist = "master..";
>>>  my $tst_only;
>>>  my $emacs = 0;
>>>  my $terse = 0;
>>> @@ -43,6 +45,7 @@ Options:
>>>    --no-tree                  run without a kernel tree
>>>    --no-signoff               do not check for 'Signed-off-by' line
>>>    --patch                    treat FILE as patchfile (default)
>>> +  --branch                   check all patches on branch since master
>>>    --emacs                    emacs compile window format
>>>    --terse                    one line per report
>>>    -f, --file                 treat FILE as regular source file
>>> @@ -69,6 +72,7 @@ GetOptions(
>>>     'tree!'         => \$tree,
>>>     'signoff!'      => \$chk_signoff,
>>>     'patch!'        => \$chk_patch,
>>> +   'branch'        => \$chk_branch,
>>>     'emacs!'        => \$emacs,
>>>     'terse!'        => \$terse,
>>>     'f|file!'       => \$file,
>>> @@ -88,9 +92,19 @@ help(0) if ($help);
>>>  
>>>  my $exit = 0;
>>>  
>>> -if ($#ARGV < 0) {
>>> -   print "$P: no input files\n";
>>> -   exit(1);
>>> +if ($chk_branch) {
>>> +   if ($#ARGV > 0) {
>>> +           print "$P: expected zero or one origni revisions\n";
>>> +           exit(1);
>>> +   }
>>> +   if ($#ARGV == 0) {
>>> +           $revlist = shift @ARGV;
>>> +   }
>>> +} else {
>>> +   if ($#ARGV < 0) {
>>> +           print "$P: no input files\n";
>>> +           exit(1);
>>> +   }
>>>  }
>>>  
>>>  my $dbg_values = 0;
>>> @@ -251,32 +265,63 @@ $chk_signoff = 0 if ($file);
>>>  my @rawlines = ();
>>>  my @lines = ();
>>>  my $vname;
>>> -for my $filename (@ARGV) {
>>> -   my $FILE;
>>> -   if ($file) {
>>> -           open($FILE, '-|', "diff -u /dev/null $filename") ||
>>> -                   die "$P: $filename: diff failed - $!\n";
>>> -   } elsif ($filename eq '-') {
>>> -           open($FILE, '<&STDIN');
>>> -   } else {
>>> -           open($FILE, '<', "$filename") ||
>>> -                   die "$P: $filename: open failed - $!\n";
>>> -   }
>>> -   if ($filename eq '-') {
>>> -           $vname = 'Your patch';
>>> -   } else {
>>> -           $vname = $filename;
>>> -   }
>>> -   while (<$FILE>) {
>>> +if ($chk_branch) {
>>> +   my @patches;
>>> +   my $HASH;
>>> +   open($HASH, "-|", "git", "log", "--format=%H", $revlist) ||
>>> +           die "$P: git log --format=%H $revlist failed - $!\n";
>>> +
>>> +   while (<$HASH>) {
>>>             chomp;
>>> -           push(@rawlines, $_);
>>> +           push @patches, $_;
>>>     }
>>> -   close($FILE);
>>> -   if (!process($filename)) {
>>> -           $exit = 1;
>>> +
>>> +   close $HASH;
>>> +
>>> +   for my $hash (@patches) {
>>> +           my $FILE;
>>> +           open($FILE, '-|', "git", "show", $hash) ||
>>> +                   die "$P: git show $hash - $!\n";
>>> +           $vname = $hash;
>>> +           while (<$FILE>) {
>>> +                   chomp;
>>> +                   push(@rawlines, $_);
>>> +           }
>>> +           close($FILE);
>>> +           if (!process($hash)) {
>>> +                   $exit = 1;
>>> +           }
>>> +           @rawlines = ();
>>> +           @lines = ();
>>> +   }
>>> +} else {
>>> +   for my $filename (@ARGV) {
>>> +           my $FILE;
>>> +           if ($file) {
>>> +                   open($FILE, '-|', "diff -u /dev/null $filename") ||
>>> +                           die "$P: $filename: diff failed - $!\n";
>>> +           } elsif ($filename eq '-') {
>>> +                   open($FILE, '<&STDIN');
>>> +           } else {
>>> +                   open($FILE, '<', "$filename") ||
>>> +                           die "$P: $filename: open failed - $!\n";
>>> +           }
>>> +           if ($filename eq '-') {
>>> +                   $vname = 'Your patch';
>>> +           } else {
>>> +                   $vname = $filename;
>>> +           }
>>> +           while (<$FILE>) {
>>> +                   chomp;
>>> +                   push(@rawlines, $_);
>>> +           }
>>> +           close($FILE);
>>> +           if (!process($filename)) {
>>> +                   $exit = 1;
>>> +           }
>>> +           @rawlines = ();
>>> +           @lines = ();
>>>     }
>>> -   @rawlines = ();
>>> -   @lines = ();
>>>  }
>>>  
>>>  exit($exit);
>>>
>>
> 
> Regards,
> Daniel
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]