From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9C0A21A1E29 for ; Sun, 16 Oct 2016 22:56:33 -0700 (PDT) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga101.fm.intel.com with ESMTP; 16 Oct 2016 22:56:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,356,1473145200"; d="scan'208";a="1045642349" Received: from ahjahagi-mobl1.amr.corp.intel.com (HELO localhost) ([10.252.140.179]) by orsmga001.jf.intel.com with ESMTP; 16 Oct 2016 22:56:02 -0700 MIME-Version: 1.0 To: "Zhu, Yonghong" , "edk2-devel@lists.01.org" Message-ID: <147668376217.29755.4387465662965357425@jljusten-ivb> From: Jordan Justen In-Reply-To: Cc: "Gao, Liming" , "Zhu, Yonghong" References: <1476350987-11628-1-git-send-email-yonghong.zhu@intel.com> <147656241940.18236.15576249978641536178@jljusten-ivb> User-Agent: alot/0.3.7 Date: Sun, 16 Oct 2016 22:56:02 -0700 Subject: Re: [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_* X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Oct 2016 05:56:33 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2016-10-16 06:32:32, Zhu, Yonghong wrote: > Thanks Jordan. I will separate the patches and send out the updated versi= on. > = One other suggestion for the commit message on your patch. Can you add this at the bottom before the Contributed-under? Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=3D113 What do you think? Maybe we should update the commit message wiki page to suggest this as well? -Jordan > > -----Original Message----- > From: Justen, Jordan L = > Sent: Sunday, October 16, 2016 4:14 AM > To: Zhu, Yonghong ; edk2-devel@lists.01.org > Cc: Gao, Liming > Subject: Re: [edk2] [Patch] BaseTools: PatchCheck to align with wiki and = report error for EFI_D_* > = > On 2016-10-13 02:29:46, Yonghong Zhu wrote: > > This patch updates PatchCheck.py=EF=BC=9A > = > Can you split this into 3 separate patches? > = > > 1.Update max line length to 70 that describe in wiki > = > I think we should update PatchCheck.py and the wiki to say that: > = > 1. The subject line of the commit message should be < 72 characters. > = > This works well with git log --oneline to still produce output less than = 80 characters. > = > 2. The other lines of the commit message should be < 76 characters. > = > When using git log, these lines are indented 4 characters, so once again = this will keep the log message readable and under 80 columns. > = > > 2.remove the two [] when do the char count calculation > = > How about instead we add a new regex to the class: > = > subject_prefix_re =3D \ > re.compile(r'''^ > \s* (\[ > [^\[\]]* # Allow all non-brackets > \])? \s* > ''', > re.VERBOSE) > = > = > Then you can use it line this: > = > self.commit_subject =3D \ > self.subject_prefix_re.sub('', self.commit_subject, 1) > = > This will change: > = > ' a' =3D> 'a' > ' [patch 1] a' =3D> 'a' > = > But, it will ignore this, because I don't think we should try to match br= ackets here: > = > ' [a [b]] a' > = > > 3.report error for EFI_D_* macro if it is used, recommend to use > > DEBUG_* format. > = > This should definitely be a separate patch since we are starting to look = at code. > = > -Jordan > = > > = > > Cc: Liming Gao > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Yonghong Zhu > > --- > > BaseTools/Scripts/PatchCheck.py | 21 ++++++++++++++++----- > > 1 file changed, 16 insertions(+), 5 deletions(-) > > = > > diff --git a/BaseTools/Scripts/PatchCheck.py = > > b/BaseTools/Scripts/PatchCheck.py index 455c130..d423220 100755 > > --- a/BaseTools/Scripts/PatchCheck.py > > +++ b/BaseTools/Scripts/PatchCheck.py > > @@ -1,9 +1,9 @@ > > ## @file > > # Check a patch for various format issues # -# Copyright (c) 2015, = > > Intel Corporation. All rights reserved.
> > +# Copyright (c) 2015 - 2016, Intel Corporation. All rights = > > +reserved.
> > # > > # This program and the accompanying materials are licensed and made = > > # available under the terms and conditions of the BSD License which = > > # accompanies this distribution. The full text of the license may be = > > # found at http://opensource.org/licenses/bsd-license.php > > @@ -14,11 +14,11 @@ > > # > > = > > from __future__ import print_function > > = > > VersionNumber =3D '0.1' > > -__copyright__ =3D "Copyright (c) 2015, Intel Corporation All rights r= eserved." > > +__copyright__ =3D "Copyright (c) 2015 - 2016, Intel Corporation All r= ights reserved." > > = > > import email > > import argparse > > import os > > import re > > @@ -195,11 +195,11 @@ class CommitMessageCheck: > > = > > if count <=3D 0: > > self.error('Empty commit message!') > > return > > = > > - if count >=3D 1 and len(lines[0]) > 76: > > + if count >=3D 1 and len(lines[0]) > 70: > > self.error('First line of commit message (subject line) ' + > > 'is too long.') > > = > > if count >=3D 1 and len(lines[0].strip()) =3D=3D 0: > > self.error('First line of commit message (subject line) ' = > > + @@ -208,11 +208,11 @@ class CommitMessageCheck: > > if count >=3D 2 and lines[1].strip() !=3D '': > > self.error('Second line of commit message should be ' + > > 'empty.') > > = > > for i in range(2, count): > > - if (len(lines[i]) > 76 and > > + if (len(lines[i]) > 70 and > > len(lines[i].split()) > 1 and > > not lines[i].startswith('git-svn-id:')): > > self.error('Line %d of commit message is too long.' % = > > (i + 1)) > > = > > last_sig_line =3D None > > @@ -354,10 +354,12 @@ class GitDiffCheck: > > line) > > if ' ' in line: > > self.added_line_error('Tab character used', line) > > if len(stripped) < len(line): > > self.added_line_error('Trailing whitespace found', line) > > + if 'EFI_D_' in line: > > + self.added_line_error('EFI_D_* used, Please use DEBUG_* = > > + format', line) > > = > > split_diff_re =3D re.compile(r''' > > (?P > > ^ diff \s+ --git \s+ a/.+ \s+ b= /.+ $ > > ) > > @@ -471,11 +473,20 @@ class CheckOnePatch: > > self.commit_msg =3D mo.group('commit_message') > > = > > self.commit_subject =3D pmail['subject'].replace('\r\n', '') > > self.commit_subject =3D self.commit_subject.replace('\n', '') > > = > > - pfx_start =3D self.commit_subject.find('[') > > + pfx_start_tmp =3D self.commit_subject.find('[') > > + pfx_end =3D self.commit_subject.find(']', pfx_start_tmp) > > + pfx_start =3D pfx_start_tmp > > + while (pfx_end > pfx_start_tmp): > > + pfx_start_tmp =3D self.commit_subject.find('[', pfx_end) > > + if pfx_start_tmp >=3D 0: > > + pfx_end =3D self.commit_subject.find(']',pfx_start_tmp) > > + else: > > + break > > + > > if pfx_start >=3D 0: > > pfx_end =3D self.commit_subject.find(']') > > if pfx_end > pfx_start: > > self.commit_prefix =3D self.commit_subject[pfx_start += 1 : pfx_end] > > self.commit_subject =3D self.commit_subject[pfx_end + = 1 = > > :].lstrip() > > -- > > 2.6.1.windows.1 > > = > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel