From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5F95C1A1E3C for ; Mon, 17 Oct 2016 23:43:01 -0700 (PDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga103.fm.intel.com with ESMTP; 17 Oct 2016 23:43:00 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,508,1473145200"; d="scan'208";a="891077512" Received: from ahjahagi-mobl1.amr.corp.intel.com (HELO localhost) ([10.252.140.179]) by orsmga003.jf.intel.com with ESMTP; 17 Oct 2016 23:43:01 -0700 MIME-Version: 1.0 To: "Zhu, Yonghong" , "edk2-devel@lists.01.org" Message-ID: <147677298023.7243.16625650400194512644@jljusten-ivb> From: Jordan Justen In-Reply-To: Cc: "Gao, Liming" , "Zhu, Yonghong" References: <1476692918-10716-1-git-send-email-yonghong.zhu@intel.com> <1476692918-10716-4-git-send-email-yonghong.zhu@intel.com> <147672349716.943.1214729718045230847@jljusten-ivb> User-Agent: alot/0.3.7 Date: Mon, 17 Oct 2016 23:43:00 -0700 Subject: Re: [Patch 3/3] BaseTools: Update PatchCheck to 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: Tue, 18 Oct 2016 06:43:01 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2016-10-17 19:02:57, Zhu, Yonghong wrote: > Hi Jordan, > = > We request to report error since the DEBUG_* is our recommend coding > style and EFI_D_ is obsolete. Looking for DEBUG and EFI_D_ since > only look for EFI_D_ may cause some False error be reported. And I > agree that user might put EFI_D_ on a separate line, however I > scanned our current edk2 code, no such usage. So I'd like to add > this support after we meet the real case. > = In that case, how about making the regex detect both? old_debug_re =3D \ re.compile(r''' DEBUG \s? \( \s? \( \s* EFI_D_ ([A-Z_]+) ''', re.VERBOSE) Then you can use something like: mo =3D self.old_debug_re.search(...) if mo is not None: self.added_line_error('EFI_D_' + mo.group(1) + ' was used, ' 'but DEBUG_' + mo.group(1) + ' is now recommended', line) -Jordan > = > -----Original Message----- > From: Justen, Jordan L = > Sent: Tuesday, October 18, 2016 12:58 AM > To: Zhu, Yonghong ; edk2-devel@lists.01.org > Cc: Gao, Liming > Subject: Re: [Patch 3/3] BaseTools: Update PatchCheck to report error for= EFI_D_* > = > On 2016-10-17 01:28:38, Yonghong Zhu wrote: > > In EDK2, DEBUG_* is recommended to be used instead of EFI_D_*. For new = > > code, they should use DEBUG_* macro. > > = > > Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=3D145 > > > = > Remove blank line. I think you meant 143, not 145. > = > Instead of looking for DEBUG and EFI_D_, I think we should just look > for EFI_D_. It is possible that the user might put the EFI_D_ on a > separate line. What do you think? > = > Should we add 'warning' support to the script first, and then flag > this as a warning rather than an error? (I don't think this is too > important, because getting an error from the script will not > currently prevent the patch from being merged.) > = > -Jordan > = > > Cc: Liming Gao > > Cc: Jordan Justen > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Yonghong Zhu > > --- > > BaseTools/Scripts/PatchCheck.py | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > = > > diff --git a/BaseTools/Scripts/PatchCheck.py = > > b/BaseTools/Scripts/PatchCheck.py index 05f8f6e..3ac0769 100755 > > --- a/BaseTools/Scripts/PatchCheck.py > > +++ b/BaseTools/Scripts/PatchCheck.py > > @@ -337,10 +337,15 @@ class GitDiffCheck: > > if self.hunk_filename is not None: > > lines.append('File: ' + self.hunk_filename) > > lines.append('Line: ' + line) > > = > > self.error(*lines) > > + > > + DEBUG_macro_re =3D re.compile(r'''^ > > + \s*DEBUG\s*\(\( > > + ''', > > + re.VERBOSE) > > = > > def check_added_line(self, line): > > eol =3D '' > > for an_eol in self.line_endings: > > if line.endswith(an_eol): > > @@ -354,10 +359,13 @@ 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 self.DEBUG_macro_re.match(line): > > + if 'EFI_D_' in line: > > + self.added_line_error('EFI_D_* format is used, = > > + recommend to use DEBUG_* format', line) > > = > > split_diff_re =3D re.compile(r''' > > (?P > > ^ diff \s+ --git \s+ a/.+ \s+ b= /.+ $ > > ) > > -- > > 2.6.1.windows.1 > >=20