From: Jordan Justen <jordan.l.justen@intel.com>
To: "Zhu, Yonghong" <yonghong.zhu@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Gao, Liming" <liming.gao@intel.com>,
"Zhu, Yonghong" <yonghong.zhu@intel.com>
Subject: Re: [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_*
Date: Sun, 16 Oct 2016 22:49:43 -0700 [thread overview]
Message-ID: <147668338371.29755.14142746503023629788@jljusten-ivb> (raw)
In-Reply-To: <B9726D6DCCFB8B4CA276A9169B02216D4F3AA725@SHSMSX103.ccr.corp.intel.com>
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
next prev parent reply other threads:[~2016-10-17 5:49 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-13 9:29 [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_* Yonghong Zhu
2016-10-13 9:29 ` [Patch] BaseTools: support PCD value to use expression in the DEC file Yonghong Zhu
2016-10-19 1:32 ` Gao, Liming
2016-10-15 20:13 ` [Patch] BaseTools: PatchCheck to align with wiki and report error for EFI_D_* Jordan Justen
2016-10-16 13:32 ` Zhu, Yonghong
2016-10-17 5:49 ` Jordan Justen [this message]
2016-10-17 6:53 ` Zhu, Yonghong
2016-10-17 5:56 ` Jordan Justen
2016-10-17 6:43 ` Zhu, Yonghong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=147668338371.29755.14142746503023629788@jljusten-ivb \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox