From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] scripts/checkpatch: Check for Reviewed-by under --strict Date: Fri, 28 Oct 2016 16:33:10 +0300 Message-ID: <87y418k38p.fsf@intel.com> References: <20161028124944.17930-1-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id D20016EB92 for ; Fri, 28 Oct 2016 13:33:15 +0000 (UTC) In-Reply-To: <20161028124944.17930-1-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , linux-kernel@vger.kernel.org Cc: Andy Whitcroft , Joe Perches , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org T24gRnJpLCAyOCBPY3QgMjAxNiwgQ2hyaXMgV2lsc29uIDxjaHJpc0BjaHJpcy13aWxzb24uY28u dWs+IHdyb3RlOgo+IFNvbWUgc3Vic3lzdGVtIHBvbGljZXMgaGF2ZSBhIHN0cmljdCByZXF1aXJl bWVudCB0aGF0IGV2ZXJ5IHBhdGNoIG11c3QKPiBoYXZlIGF0IGxlYXN0IG9uZSByZXZpZXdlciBi ZWZvcmUgYmVpbmcgYXBwcm92ZWQgZm9yIHVwc3RyZWFtLiBTaW5jZQo+IGVuY291cmFnaW5nIHJl dmlldyBpcyBnb29kIHBvbGljeSAoZ3JlYXQgcmV2aWV3IGlzIGV2ZW4gYmV0dGVyIHBvbGljeSEp Cj4gZW5mb3JjZSBjaGVja2luZyBmb3IgYSBSZXZpZXdlZC1ieSB3aGVuIGNoZWNrcGF0aCBpcyBy dW4gd2l0aCAtLXN0cmljdAo+IChvciB3aXRoIC0tcmV2aWV3KS4KCkhtbSwgZG8geW91IGltcGx5 IHRoZSBtYWludGFpbmVyIHdvdWxkIGhhdmUgdG8gYWRkIGhpcyBSZXZpZXdlZC1ieSBpbgphZGRp dGlvbiB0byBTaWduZWQtb2ZmLWJ5PyBJIGZpbmQgdGhhdCBhIGJpdCB0b28gbXVjaCAoZXNwZWNp YWxseSBpZiB5b3UKaW50ZW5kIHRvIGVuZm9yY2UgdGhpcyBvdmVyIGF0IG91ciBjb3JuZXIgb2Yg dGhlIGtlcm5lbCA7KQoKQlIsCkphbmkuCgoKPgo+IFNpZ25lZC1vZmYtYnk6IENocmlzIFdpbHNv biA8Y2hyaXNAY2hyaXMtd2lsc29uLmNvLnVrPgo+IENjOiBBbmR5IFdoaXRjcm9mdCA8YXB3QGNh bm9uaWNhbC5jb20+Cj4gQ2M6IEpvZSBQZXJjaGVzIDxqb2VAcGVyY2hlcy5jb20+Cj4gQ2M6IEpv b25hcyBMYWh0aW5lbiA8am9vbmFzLmxhaHRpbmVuQGxpbnV4LmludGVsLmNvbT4KPiAtLS0KPiAg c2NyaXB0cy9jaGVja3BhdGNoLnBsIHwgMTcgKysrKysrKysrKysrKysrKy0KPiAgMSBmaWxlIGNo YW5nZWQsIDE2IGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkKPgo+IGRpZmYgLS1naXQgYS9z Y3JpcHRzL2NoZWNrcGF0Y2gucGwgYi9zY3JpcHRzL2NoZWNrcGF0Y2gucGwKPiBpbmRleCBhODM2 OGQxYzQzNDguLjllYWE1YTRmYmJjMCAxMDA3NTUKPiAtLS0gYS9zY3JpcHRzL2NoZWNrcGF0Y2gu cGwKPiArKysgYi9zY3JpcHRzL2NoZWNrcGF0Y2gucGwKPiBAQCAtMjEsNiArMjEsNyBAQCB1c2Ug R2V0b3B0OjpMb25nIHF3KDpjb25maWcgbm9fYXV0b19hYmJyZXYpOwo+ICBteSAkcXVpZXQgPSAw Owo+ICBteSAkdHJlZSA9IDE7Cj4gIG15ICRjaGtfc2lnbm9mZiA9IDE7Cj4gK215ICRjaGtfcmV2 aWV3ID0gMDsKPiAgbXkgJGNoa19wYXRjaCA9IDE7Cj4gIG15ICR0c3Rfb25seTsKPiAgbXkgJGVt YWNzID0gMDsKPiBAQCAtNjksNiArNzAsNyBAQCBPcHRpb25zOgo+ICAgIC1xLCAtLXF1aWV0ICAg ICAgICAgICAgICAgIHF1aWV0Cj4gICAgLS1uby10cmVlICAgICAgICAgICAgICAgICAgcnVuIHdp dGhvdXQgYSBrZXJuZWwgdHJlZQo+ICAgIC0tbm8tc2lnbm9mZiAgICAgICAgICAgICAgIGRvIG5v dCBjaGVjayBmb3IgJ1NpZ25lZC1vZmYtYnknIGxpbmUKPiArICAtLXJldmlldyAgICAgICAgICAg ICAgICAgICBjaGVjayBmb3IgJ1Jldmlld2VkLWJ5JyBsaW5lCj4gICAgLS1wYXRjaCAgICAgICAg ICAgICAgICAgICAgdHJlYXQgRklMRSBhcyBwYXRjaGZpbGUgKGRlZmF1bHQpCj4gICAgLS1lbWFj cyAgICAgICAgICAgICAgICAgICAgZW1hY3MgY29tcGlsZSB3aW5kb3cgZm9ybWF0Cj4gICAgLS10 ZXJzZSAgICAgICAgICAgICAgICAgICAgb25lIGxpbmUgcGVyIHJlcG9ydAo+IEBAIC0xODMsNiAr MTg1LDcgQEAgR2V0T3B0aW9ucygKPiAgCSdxfHF1aWV0KycJPT4gXCRxdWlldCwKPiAgCSd0cmVl IScJCT0+IFwkdHJlZSwKPiAgCSdzaWdub2ZmIScJPT4gXCRjaGtfc2lnbm9mZiwKPiArCSdyZXZp ZXchJwk9PiBcJGNoa19yZXZpZXcsCj4gIAkncGF0Y2ghJwk9PiBcJGNoa19wYXRjaCwKPiAgCSdl bWFjcyEnCT0+IFwkZW1hY3MsCj4gIAkndGVyc2UhJwk9PiBcJHRlcnNlLAo+IEBAIC0yMTcsNyAr MjIwLDcgQEAgaGVscCgwKSBpZiAoJGhlbHApOwo+ICAKPiAgbGlzdF90eXBlcygwKSBpZiAoJGxp c3RfdHlwZXMpOwo+ICAKPiAtJGZpeCA9IDEgaWYgKCRmaXhfaW5wbGFjZSk7Cj4gKyRjaGtfcmV2 aWV3ID0gMSBpZiAoJGNoZWNrKTsgIyAtLXN0cmljdCBpbXBsaWVzIGNoZWNraW5nIGZvciBSZXZp ZXdlZC1ieQo+ICAkY2hlY2tfb3JpZyA9ICRjaGVjazsKPiAgCj4gIG15ICRleGl0ID0gMDsKPiBA QCAtODU3LDYgKzg2MCw3IEBAIHN1YiBnaXRfY29tbWl0X2luZm8gewo+ICB9Cj4gIAo+ICAkY2hr X3NpZ25vZmYgPSAwIGlmICgkZmlsZSk7Cj4gKyRjaGtfcmV2aWV3ID0gMCBpZiAoJGZpbGUpOwo+ ICAKPiAgbXkgQHJhd2xpbmVzID0gKCk7Cj4gIG15IEBsaW5lcyA9ICgpOwo+IEBAIC0yMTMwLDYg KzIxMzQsNyBAQCBzdWIgcHJvY2VzcyB7Cj4gIAo+ICAJb3VyICRjbGVhbiA9IDE7Cj4gIAlteSAk c2lnbm9mZiA9IDA7Cj4gKwlteSAkcmV2aWV3ID0gMDsKPiAgCW15ICRpc19wYXRjaCA9IDA7Cj4g IAlteSAkaW5faGVhZGVyX2xpbmVzID0gJGZpbGUgPyAwIDogMTsKPiAgCW15ICRpbl9jb21taXRf bG9nID0gMDsJCSNTY2FubmluZyBsaW5lcyBiZWZvcmUgcGF0Y2gKPiBAQCAtMjQwMCw2ICsyNDA1 LDEyIEBAIHN1YiBwcm9jZXNzIHsKPiAgCQkJJGluX2NvbW1pdF9sb2cgPSAwOwo+ICAJCX0KPiAg Cj4gKyMgQ2hlY2sgdGhlIHBhdGNoIGZvciBhbnkgcmV2aWV3Ogo+ICsJCWlmICgkbGluZSA9fiAv XlxzKnJldmlld2VkLWJ5Oi9pKSB7Cj4gKwkJCSRyZXZpZXcrKzsKPiArCQkJJGluX2NvbW1pdF9s b2cgPSAwOwo+ICsJCX0KPiArCj4gICMgQ2hlY2sgaWYgTUFJTlRBSU5FUlMgaXMgYmVpbmcgdXBk YXRlZC4gIElmIHNvLCB0aGVyZSdzIHByb2JhYmx5IG5vIG5lZWQgdG8KPiAgIyBlbWl0IHRoZSAi ZG9lcyBNQUlOVEFJTkVSUyBuZWVkIHVwZGF0aW5nPyIgbWVzc2FnZSBvbiBmaWxlIGFkZC9tb3Zl L2RlbGV0ZQo+ICAJCWlmICgkbGluZSA9fiAvXlxzKk1BSU5UQUlORVJTXHMqXHwvKSB7Cj4gQEAg LTYyMDQsNiArNjIxNSwxMCBAQCBzdWIgcHJvY2VzcyB7Cj4gIAkJRVJST1IoIk1JU1NJTkdfU0lH Tl9PRkYiLAo+ICAJCSAgICAgICJNaXNzaW5nIFNpZ25lZC1vZmYtYnk6IGxpbmUocylcbiIpOwo+ ICAJfQo+ICsJaWYgKCRpc19wYXRjaCAmJiAkaGFzX2NvbW1pdF9sb2cgJiYgJGNoa19yZXZpZXcg JiYgJHJldmlldyA9PSAwKSB7Cj4gKwkJRVJST1IoIk1JU1NJTkdfUkVWSUVXIiwKPiArCQkgICAg ICAiTWlzc2luZyBSZXZpZXdlZC1ieTogbGluZShzKVxuIik7Cj4gKwl9Cj4gIAo+ICAJcHJpbnQg cmVwb3J0X2R1bXAoKTsKPiAgCWlmICgkc3VtbWFyeSAmJiAhKCRjbGVhbiA9PSAxICYmICRxdWll dCA9PSAxKSkgewoKLS0gCkphbmkgTmlrdWxhLCBJbnRlbCBPcGVuIFNvdXJjZSBUZWNobm9sb2d5 IENlbnRlcgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpJ bnRlbC1nZnggbWFpbGluZyBsaXN0CkludGVsLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0 cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759925AbcJ1NdW (ORCPT ); Fri, 28 Oct 2016 09:33:22 -0400 Received: from mga01.intel.com ([192.55.52.88]:46389 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754599AbcJ1NdV (ORCPT ); Fri, 28 Oct 2016 09:33:21 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,410,1473145200"; d="scan'208";a="779054646" From: Jani Nikula To: Chris Wilson , linux-kernel@vger.kernel.org Cc: Andy Whitcroft , Joe Perches , intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] scripts/checkpatch: Check for Reviewed-by under --strict In-Reply-To: <20161028124944.17930-1-chris@chris-wilson.co.uk> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20161028124944.17930-1-chris@chris-wilson.co.uk> Date: Fri, 28 Oct 2016 16:33:10 +0300 Message-ID: <87y418k38p.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 28 Oct 2016, Chris Wilson wrote: > Some subsystem polices have a strict requirement that every patch must > have at least one reviewer before being approved for upstream. Since > encouraging review is good policy (great review is even better policy!) > enforce checking for a Reviewed-by when checkpath is run with --strict > (or with --review). Hmm, do you imply the maintainer would have to add his Reviewed-by in addition to Signed-off-by? I find that a bit too much (especially if you intend to enforce this over at our corner of the kernel ;) BR, Jani. > > Signed-off-by: Chris Wilson > Cc: Andy Whitcroft > Cc: Joe Perches > Cc: Joonas Lahtinen > --- > scripts/checkpatch.pl | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index a8368d1c4348..9eaa5a4fbbc0 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -21,6 +21,7 @@ use Getopt::Long qw(:config no_auto_abbrev); > my $quiet = 0; > my $tree = 1; > my $chk_signoff = 1; > +my $chk_review = 0; > my $chk_patch = 1; > my $tst_only; > my $emacs = 0; > @@ -69,6 +70,7 @@ Options: > -q, --quiet quiet > --no-tree run without a kernel tree > --no-signoff do not check for 'Signed-off-by' line > + --review check for 'Reviewed-by' line > --patch treat FILE as patchfile (default) > --emacs emacs compile window format > --terse one line per report > @@ -183,6 +185,7 @@ GetOptions( > 'q|quiet+' => \$quiet, > 'tree!' => \$tree, > 'signoff!' => \$chk_signoff, > + 'review!' => \$chk_review, > 'patch!' => \$chk_patch, > 'emacs!' => \$emacs, > 'terse!' => \$terse, > @@ -217,7 +220,7 @@ help(0) if ($help); > > list_types(0) if ($list_types); > > -$fix = 1 if ($fix_inplace); > +$chk_review = 1 if ($check); # --strict implies checking for Reviewed-by > $check_orig = $check; > > my $exit = 0; > @@ -857,6 +860,7 @@ sub git_commit_info { > } > > $chk_signoff = 0 if ($file); > +$chk_review = 0 if ($file); > > my @rawlines = (); > my @lines = (); > @@ -2130,6 +2134,7 @@ sub process { > > our $clean = 1; > my $signoff = 0; > + my $review = 0; > my $is_patch = 0; > my $in_header_lines = $file ? 0 : 1; > my $in_commit_log = 0; #Scanning lines before patch > @@ -2400,6 +2405,12 @@ sub process { > $in_commit_log = 0; > } > > +# Check the patch for any review: > + if ($line =~ /^\s*reviewed-by:/i) { > + $review++; > + $in_commit_log = 0; > + } > + > # Check if MAINTAINERS is being updated. If so, there's probably no need to > # emit the "does MAINTAINERS need updating?" message on file add/move/delete > if ($line =~ /^\s*MAINTAINERS\s*\|/) { > @@ -6204,6 +6215,10 @@ sub process { > ERROR("MISSING_SIGN_OFF", > "Missing Signed-off-by: line(s)\n"); > } > + if ($is_patch && $has_commit_log && $chk_review && $review == 0) { > + ERROR("MISSING_REVIEW", > + "Missing Reviewed-by: line(s)\n"); > + } > > print report_dump(); > if ($summary && !($clean == 1 && $quiet == 1)) { -- Jani Nikula, Intel Open Source Technology Center