Re: [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*

["Zhu, Yonghong" <yonghong.zhu@intel.com>]

It's good to me. I will send patch to align with this wiki.

Best Regards,
Zhu Yonghong


-----Original Message-----
From: Justen, Jordan L 
Sent: Monday, October 17, 2016 1:50 PM
To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: RE: [edk2] [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*

On 2016-10-16 06:32:32, Zhu, Yonghong wrote:
> Thanks Jordan. I will separate the patches and send out the updated version.
> 
> Besides can you help to update the wiki ? 
> 

How does it look?

https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format

-Jordan

> 
> -----Original Message-----
> From: Justen, Jordan L
> Sent: Sunday, October 16, 2016 4:14 AM
> To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>
> 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：
> 
> 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 = \
>         re.compile(r'''^
>                            \s* (\[
>                                [^\[\]]* # Allow all non-brackets
>                            \])? \s*
>                    ''',
>                    re.VERBOSE)
> 
> 
> Then you can use it line this:
> 
>           self.commit_subject = \
>               self.subject_prefix_re.sub('', self.commit_subject, 1)
> 
> This will change:
> 
>   ' a' => 'a'
>   ' [patch 1] a' => 'a'
> 
> But, it will ignore this, because I don't think we should try to match brackets 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 <liming.gao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
> > ---
> >  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.<BR>
> > +#  Copyright (c) 2015 - 2016, Intel Corporation. All rights 
> > +reserved.<BR>
> >  #
> >  #  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 = '0.1'
> > -__copyright__ = "Copyright (c) 2015, Intel Corporation  All rights reserved."
> > +__copyright__ = "Copyright (c) 2015 - 2016, Intel Corporation  All rights reserved."
> >  
> >  import email
> >  import argparse
> >  import os
> >  import re
> > @@ -195,11 +195,11 @@ class CommitMessageCheck:
> >  
> >          if count <= 0:
> >              self.error('Empty commit message!')
> >              return
> >  
> > -        if count >= 1 and len(lines[0]) > 76:
> > +        if count >= 1 and len(lines[0]) > 70:
> >              self.error('First line of commit message (subject line) ' +
> >                         'is too long.')
> >  
> >          if count >= 1 and len(lines[0].strip()) == 0:
> >              self.error('First line of commit message (subject line) ' 
> > + @@ -208,11 +208,11 @@ class CommitMessageCheck:
> >          if count >= 2 and lines[1].strip() != '':
> >              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 = 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 = re.compile(r'''
> >                                     (?P<cmd>
> >                                         ^ diff \s+ --git \s+ a/.+ \s+ b/.+ $
> >                                     ) @@ -471,11 +473,20 @@ class 
> > CheckOnePatch:
> >              self.commit_msg = mo.group('commit_message')
> >  
> >          self.commit_subject = pmail['subject'].replace('\r\n', '')
> >          self.commit_subject = self.commit_subject.replace('\n', '')
> >  
> > -        pfx_start = self.commit_subject.find('[')
> > +        pfx_start_tmp = self.commit_subject.find('[')
> > +        pfx_end = self.commit_subject.find(']', pfx_start_tmp)
> > +        pfx_start = pfx_start_tmp
> > +        while (pfx_end > pfx_start_tmp):
> > +            pfx_start_tmp = self.commit_subject.find('[', pfx_end)
> > +            if pfx_start_tmp >= 0:
> > +                pfx_end = self.commit_subject.find(']',pfx_start_tmp)
> > +            else:
> > +                break
> > +
> >          if pfx_start >= 0:
> >              pfx_end = self.commit_subject.find(']')
> >              if pfx_end > pfx_start:
> >                  self.commit_prefix = self.commit_subject[pfx_start + 1 : pfx_end]
> >                  self.commit_subject = 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